On 03/08/2017 10:37 PM, Minchan Kim wrote:
>[...]
I think it's the matter of taste.
if (try_to_unmap(xxx))
something
else
something
It's perfectly understandable to me. IOW, if try_to_unmap returns true,
it means it did unmap successfully. Otherwise, failed.
IMHO, SWAP_SUCCESS or TTU_RESULT_* seems to be an over-engineering.
If the user want it, user can do it by introducing right variable name
in his context. See below.
I'm OK with that approach. Just something to avoid the "what does !ret mean in this
function call" is what I was looking for...
[...]
forcekill = PageDirty(hpage) || (flags & MF_MUST_KILL);
- kill_procs(&tokill, forcekill, trapno,
- ret != SWAP_SUCCESS, p, pfn, flags);
+ kill_procs(&tokill, forcekill, trapno, !ret , p, pfn, flags);
The kill_procs() invocation was a little more readable before.
Indeed but I think it's not a problem of try_to_unmap but ret variable name
isn't good any more. How about this?
bool unmap_success;
unmap_success = try_to_unmap(hpage, ttu);
..
kill_procs(&tokill, forcekill, trapno, !unmap_success , p, pfn, flags);
..
return unmap_success;
My point is user can introduce whatever variable name depends on his
context. No need to make return variable complicated, IMHO.
Yes, the local variable basically achieves what I was hoping for, so sure, works for
me.
[...]
- case SWAP_FAIL:
Again: the SWAP_FAIL makes it crystal clear which case we're in.
To me, I don't feel it.
To me, below is perfectly understandable.
if (try_to_unmap())
do something
That's why I think it's matter of taste. Okay, I admit I might be
biased, too so I will consider what you suggested if others votes
it.
Yes, if it's really just a matter of taste, then not worth debating. Your change
above is fine I think.
thanks
john h
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
