lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Feb 2020 10:31:28 +0100 From: Jan Kara <jack@...e.cz> To: Minchan Kim <minchan@...nel.org> Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>, Jan Kara <jack@...e.cz>, Matthew Wilcox <willy@...radead.org>, Josef Bacik <josef@...icpanda.com>, Johannes Weiner <hannes@...xchg.org>, Robert Stupp <snazy@....de> Subject: Re: [PATCH 3/3] mm: make PageReadahead more strict On Wed 12-02-20 14:16:14, Minchan Kim wrote: > PG_readahead flag is shared with PG_reclaim but PG_reclaim is only > used in write context while PG_readahead is used for read context. > > To make it clear, let's introduce PageReadahead wrapper with > !PageWriteback so it could make code clear and we could drop > PageWriteback check in page_cache_async_readahead, which removes > pointless dropping mmap_sem. > > Signed-off-by: Minchan Kim <minchan@...nel.org> ... > +/* Clear PG_readahead only if it's PG_readahead, not PG_reclaim */ > +static inline int TestClearPageReadahead(struct page *page) > +{ > + VM_BUG_ON_PGFLAGS(PageCompound(page), page); > + > + return !PageWriteback(page) || > + test_and_clear_bit(PG_reclaim, &page->flags); > +} I think this is still wrong - if PageWriteback is not set, it will never clear PG_reclaim bit so effectively the page will stay in PageReadahead state! The logic you really want to implement is: if (PageReadahead(page)) { <- this is your new PageReadahead implementation clear_bit(PG_reclaim, &page->flags); return 1; } return 0; Now this has the problem that it is not atomic. The only way I see to make this fully atomic is using cmpxchg(). If we wanted to make this kinda-sorta OK, the proper condition would look like: return !PageWriteback(page) **&&** test_and_clear_bit(PG_reclaim, &page->flags); Which is similar to what you originally had but different because in C '&&' operator is not commutative due to side-effects committed at sequence points. BTW: I share Andrew's view that we are piling hacks to fix problems caused by older hacks. But I don't see any good option how to unalias PG_readahead and PG_reclaim. Honza -- Jan Kara <jack@...e.com> SUSE Labs, CR
Powered by blists - more mailing lists