On Mon, Jan 24, 2022 at 03:14:41PM +0000, David Howells wrote: > Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > Questions for Willy: > > > - Can we get a function to return the number of pages in a readahead > > > batch? > > > > > > - Can we get a function to commit a readahead batch? Currently, this is > > > done when we call __readahead_batch(), but that means ractl->_nr_pages > > > isn't up to date at the point we need it to be. However, we want to > > > check to see if the ractl is empty, then get server credits and only > > > *then* call __readahead_batch() as we don't know how big a batch we're > > > allowed till we have the credits. > > > > If you insist on using the primitives in a way that nobody else uses > > them, you're going to find they don't work. What's wrong with the > > way that FUSE uses them in fuse_readahead()? > > You mean doing this? > > nr_pages = readahead_count(rac) - nr_pages; > > that would seem to indicate that the readahead interface is wrong. Why should > readahead_count() need correction? I think I can see *why* the batching stuff > is hidden, but it seems that the comment for readahead_count() needs to > mention this if you aren't going to fix it. > > Would it be possible to make readahead_count() do: > > return rac->_nr_pages - rac->_batch_count; > > maybe? Yes, I think that would make sense. Do we also need to change readhead_length()? It seems to me that it's only ever called once at initialisation, so it should be possible to keep the two in sync. Can you write & test such a patch? I'll support it going upstream (either by taking it myself or giving you a R-b to take it through your tree). -- Linux-cachefs mailing list Linux-cachefs@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/linux-cachefs
