Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1614110

Re: [PATCH -mm -v7 4/9] mm, THP, swap: Add get_huge_swap_page()

Show all headers | View raw

On Thu, Mar 30, 2017 at 12:28:17PM +0800, Huang, Ying wrote:
> Johannes Weiner <hannes@cmpxchg.org> writes:
> > On Tue, Mar 28, 2017 at 01:32:04PM +0800, Huang, Ying wrote:
> >> @@ -527,6 +527,23 @@ static inline swp_entry_t get_swap_page(void)
> >>  
> >>  #endif /* CONFIG_SWAP */
> >>  
> >> +#ifdef CONFIG_THP_SWAP_CLUSTER
> >> +static inline swp_entry_t get_huge_swap_page(void)
> >> +{
> >> +	swp_entry_t entry;
> >> +
> >> +	if (get_swap_pages(1, &entry, true))
> >> +		return entry;
> >> +	else
> >> +		return (swp_entry_t) {0};
> >> +}
> >> +#else
> >> +static inline swp_entry_t get_huge_swap_page(void)
> >> +{
> >> +	return (swp_entry_t) {0};
> >> +}
> >> +#endif
> >
> > Your introducing a function without a user, making it very hard to
> > judge whether the API is well-designed for the callers or not.
> >
> > I pointed this out as a systemic problem with this patch series in v3,
> > along with other stuff, but with the way this series is structured I'm
> > having a hard time seeing whether you implemented my other feedback or
> > whether your counter arguments to them are justified.
> >
> > I cannot review and ack these patches this way.
> 
> Sorry for inconvenience, I will send a new version to combine the
> function definition and usage into one patch at least for you to
> review.

We tried this before. I reviewed the self-contained patch and you
incorporated the feedback into the split-out structure that made it
impossible for me to verify the updates.

I'm not sure why you insist on preserving this series format. It's not
good for review, and it's not good for merging and git history.

> But I think we can continue our discussion in the comments your
> raised so far firstly, what do you think about that?

Yeah, let's finish the discussions before -v8.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH -mm -v7 0/9] THP swap: Delay splitting THP during swapping out "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
  [PATCH -mm -v7 6/9] mm, THP, swap: Support to add/delete THP to/from swap cache "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
  [PATCH -mm -v7 2/9] mm, memcg: Support to charge/uncharge multiple swap entries "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
    Re: [PATCH -mm -v7 2/9] mm, memcg: Support to charge/uncharge  multiple swap entries Johannes Weiner <hannes@cmpxchg.org> - 2017-03-29 19:10 +0200
      Re: [PATCH -mm -v7 2/9] mm, memcg: Support to charge/uncharge multiple swap entries "Huang\, Ying" <ying.huang@intel.com> - 2017-03-30 03:00 +0200
        Re: [PATCH -mm -v7 2/9] mm, memcg: Support to charge/uncharge  multiple swap entries Johannes Weiner <hannes@cmpxchg.org> - 2017-03-31 17:00 +0200
  [PATCH -mm -v7 4/9] mm, THP, swap: Add get_huge_swap_page() "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
    Re: [PATCH -mm -v7 4/9] mm, THP, swap: Add get_huge_swap_page() Johannes Weiner <hannes@cmpxchg.org> - 2017-03-29 19:10 +0200
      Re: [PATCH -mm -v7 4/9] mm, THP, swap: Add get_huge_swap_page() "Huang\, Ying" <ying.huang@intel.com> - 2017-03-30 06:30 +0200
        Re: [PATCH -mm -v7 4/9] mm, THP, swap: Add get_huge_swap_page() Johannes Weiner <hannes@cmpxchg.org> - 2017-03-31 17:30 +0200
          Re: [PATCH -mm -v7 4/9] mm, THP, swap: Add get_huge_swap_page() "Huang\, Ying" <ying.huang@intel.com> - 2017-04-01 05:40 +0200
  [PATCH -mm -v7 7/9] mm, THP: Add can_split_huge_page() "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
  [PATCH -mm -v7 3/9] mm, THP, swap: Add swap cluster allocate/free functions "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
  [PATCH -mm -v7 5/9] mm, THP, swap: Support to clear SWAP_HAS_CACHE for huge page "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
  [PATCH -mm -v7 9/9] mm, THP, swap: Delay splitting THP during swap out "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
    Re: [PATCH -mm -v7 9/9] mm, THP, swap: Delay splitting THP during  swap out Johannes Weiner <hannes@cmpxchg.org> - 2017-03-29 19:20 +0200
      Re: [PATCH -mm -v7 9/9] mm, THP, swap: Delay splitting THP during swap out "Huang\, Ying" <ying.huang@intel.com> - 2017-03-30 06:20 +0200
        Re: [PATCH -mm -v7 9/9] mm, THP, swap: Delay splitting THP during  swap out Johannes Weiner <hannes@cmpxchg.org> - 2017-03-31 17:00 +0200
          Re: [PATCH -mm -v7 9/9] mm, THP, swap: Delay splitting THP during swap out "Huang\, Ying" <ying.huang@intel.com> - 2017-04-01 05:00 +0200
  [PATCH -mm -v7 8/9] mm, THP, swap: Support to split THP in swap cache "Huang, Ying" <ying.huang@intel.com> - 2017-03-28 07:40 +0200
  Re: [PATCH -mm -v7 0/9] THP swap: Delay splitting THP during  swapping out Andrew Morton <akpm@linux-foundation.org> - 2017-03-29 00:20 +0200
    Re: [PATCH -mm -v7 0/9] THP swap: Delay splitting THP during swapping out "Huang\, Ying" <ying.huang@intel.com> - 2017-03-29 11:00 +0200

csiph-web