On 04/18/2017 09:51 PM, Nikolay Borisov wrote: > > > On 14.04.2017 17:07, Andrey Ryabinin wrote: >> invalidate_bdev() calls cleancache_invalidate_inode() iff ->nrpages != 0 >> which doen't make any sense. >> Make invalidate_bdev() always invalidate cleancache data. >> >> Fixes: c515e1fd361c ("mm/fs: add hooks to support cleancache") >> Signed-off-by: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx> >> --- >> fs/block_dev.c | 11 +++++------ >> 1 file changed, 5 insertions(+), 6 deletions(-) >> >> diff --git a/fs/block_dev.c b/fs/block_dev.c >> index e405d8e..7af4787 100644 >> --- a/fs/block_dev.c >> +++ b/fs/block_dev.c >> @@ -103,12 +103,11 @@ void invalidate_bdev(struct block_device *bdev) >> { >> struct address_space *mapping = bdev->bd_inode->i_mapping; >> >> - if (mapping->nrpages == 0) >> - return; >> - >> - invalidate_bh_lrus(); >> - lru_add_drain_all(); /* make sure all lru add caches are flushed */ >> - invalidate_mapping_pages(mapping, 0, -1); >> + if (mapping->nrpages) { >> + invalidate_bh_lrus(); >> + lru_add_drain_all(); /* make sure all lru add caches are flushed */ >> + invalidate_mapping_pages(mapping, 0, -1); >> + } > > How is this different than the current code? You will only invalidate > the mapping iff ->nrpages > 0 ( I assume it can't go down below 0) ? The difference is that invalidate_bdev() now always calls cleancache_invalidate_inode() (you won't see it in this diff, it's placed after this if(mapping->nrpages){} block,) > Perhaps just remove the if altogether? > Given that invalidate_mapping_pages() invalidates exceptional entries as well, it certainly doesn't look right that we look only at mapping->nrpages and completely ignore ->nrexceptional. So maybe removing if() would be a right thing to do. But I think that should be a separate patch as it would fix a another bug probably introduced by commit 91b0abe36a7b ("mm + fs: store shadow entries in page cache") My intention here was to fix only cleancache case. >> /* 99% of the time, we don't need to flush the cleancache on the bdev. >> * But, for the strange corners, lets be cautious >> */ >> -- 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>