On Fri, May 19, 2017 at 04:48:54PM -0400, Zi Yan wrote:

On 19 May 2017, at 16:28, Mel Gorman wrote:

quoted

On Fri, May 19, 2017 at 12:37:38PM -0400, Zi Yan wrote:

quoted

quoted

As you say, there is no functional change but the helper name is vague
and gives no hint to what's it's checking for. It's somewhat tolerable as
it is as it's obvious what is being checked but the same is not true with
the helper name.

Does queue_pages_invert_nodemask_check() work? I can change the helper name
in the next version.

Not particularly, maybe queue_pages_required and invert the check with a
comment above it explaining what it's checking for would be ok.

queue_pages_required() is too broad,

I'm somewhat amused that you'd complain that "required" is too broad while
thinking "check" is somehow self-explanatory.

I would take queue_pages_page_nid_check()
and invert the check with a comment above saying

/*
 * Check if the page's nid is in qp->nmask.
 *
 * If MPOL_MF_INVERT is set in qp->flags, check if the nid is
 * in the invert of qp->nmask.
 */

Does it work?

I still don't like the name but I also am not interested in debating it
further for something so small. Add the comment, it's better than nothing.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>