Nick Piggin <npiggin@xxxxxxx> wrote: > > The problem is that the code called assumes that the struct page * > > argument points to a single page, not an array of pages as would > > presumably be the case if PAGE_CACHE_SIZE > PAGE_SIZE. > > Incorrect. Christoph's patch for example does this by using compound pages. > Now I personally don't like the patch or see the point in PAGE_CACHE_SIZE / > PAGE_SIZE distinction, but I'm just telling you what the convention is. There > is no point you arguing against it, that's simply how it is. No! You are wrong. I wrote the AFS code. I *know* it can only deal with single pages. It has no knowledge of compound pages and does not handle page arrays. This may be a flaw in my code, but it's there nonetheless. The assertion is a guard against that. *That* was the point of my statement. Perhaps I should've said 'my code' rather than 'the code'. If Christoph has a patch to deal with that, it's either not upstream yet or it hasn't altered AFS. > > So: you may not change the assertion unless you also fix the lower > > functions. > > I won't change the assertion, because you haven't been following said > convention, so just changing it in one place is stupider than not changing > it at all, but not for the reason cited. The convention is not precisely clear. Just grep for PAGE_CACHE_SIZE in Documentation/. It's only mentioned twice, and in neither case does it give any information about what PAGE_CACHE_SIZE is, what it represents, or where it applies. Therefore it's an ill-defined concept. If you look in Documentation/filesystems/vfs.txt, you'll see that it almost always talks about 'pages'. It only mentions 'pagecache pages' once - in the description of write_begin(), but it's not clear whether that means anything. However, I've now noted that I need to fix my code, so just keep the assertion for now and I'll fix my code to handle multipage blocks. David - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
