David Howells wrote:
> > > - wait_on_page_writeback(page);
> > > + if (wait_on_page_writeback_killable(page) < 0)
> > > + return VM_FAULT_RETRY | VM_FAULT_LOCKED;
> >
> > You forgot to unlock the page.
>
> Do I need to? Doesn't VM_FAULT_LOCKED indicate that to the caller? Or is it
>
Matthew Wilcox wrote:
> On Tue, Mar 23, 2021 at 01:17:20PM +, David Howells wrote:
> > +++ b/fs/afs/write.c
> > @@ -846,7 +846,7 @@ vm_fault_t afs_page_mkwrite(struct vm_fault *vmf)
> > */
> > #ifdef CONFIG_AFS_FSCACHE
> > if (PageFsCache(page) &&
> > -
On Tue, Mar 23, 2021 at 01:17:20PM +, David Howells wrote:
> +++ b/fs/afs/write.c
> @@ -846,7 +846,7 @@ vm_fault_t afs_page_mkwrite(struct vm_fault *vmf)
>*/
> #ifdef CONFIG_AFS_FSCACHE
> if (PageFsCache(page) &&
> - wait_on_page_bit_killable(page, PG_fscache) < 0)
> +
David Howells wrote:
> Matthew Wilcox wrote:
>
> > That also brings up that there is no set_page_private_2(). I think
> > that's OK -- you only set PageFsCache() immediately after reading the
> > page from the server. But I feel this "unlock_page_private_2" is actually
> >
Matthew Wilcox wrote:
> That also brings up that there is no set_page_private_2(). I think
> that's OK -- you only set PageFsCache() immediately after reading the
> page from the server. But I feel this "unlock_page_private_2" is actually
> "clear_page_private_2" -- ie it's equivalent to
On Wed, Mar 10, 2021 at 04:54:49PM +, David Howells wrote:
> Add a function, unlock_page_private_2(), to unlock PG_private_2 analogous
> to that of PG_lock. Add a kerneldoc banner to that indicating the example
> usage case.
One of the things which confused me about this was ... where's the
David Howells wrote:
> (1) For the old fscache code that I'm trying to phase out, it does not take a
> ref when PG_fscache is taken (probably incorrectly), relying instead on
> releasepage, etc. getting called to strip the PG_fscache bit. PG_fscache
> is held for the lifetime of
Linus Torvalds wrote:
> And as far as I can tell, fscache doesn't want that PG_private_2 bit
> to interact with the random VM lifetime or migration rules either, and
> should rely entirely on the page count. David?
It's slightly complicated for fscache as there are two separate pieces of code
On Tue, Mar 16, 2021 at 7:12 PM Josef Bacik wrote:
>
>
> Yeah it's just a flag, we use it to tell that the page is part of a range that
> has been allocated for IO. The lifetime of the page is independent of the
> page,
> but is generally either dirty or under writeback, so either it goes
On 3/16/21 8:43 PM, Linus Torvalds wrote:
[ Adding btrfs people explicitly, maybe they see this on the fs-devel
list, but maybe they don't react .. ]
On Tue, Mar 16, 2021 at 12:07 PM Matthew Wilcox wrote:
This isn't a problem with this patch per se, but I'm concerned about
private2 and
[ Adding btrfs people explicitly, maybe they see this on the fs-devel
list, but maybe they don't react .. ]
On Tue, Mar 16, 2021 at 12:07 PM Matthew Wilcox wrote:
>
> This isn't a problem with this patch per se, but I'm concerned about
> private2 and expected page refcounts.
Ugh. You are very
On Tue, Mar 16, 2021 at 08:38:00PM +, David Howells wrote:
> Matthew Wilcox wrote:
>
> > So ... a page with both flags cleared should have a refcount of N.
> > A page with one or both flags set should have a refcount of N+1.
> > ...
> > How is a poor filesystem supposed to make that true?
Matthew Wilcox wrote:
> So ... a page with both flags cleared should have a refcount of N.
> A page with one or both flags set should have a refcount of N+1.
> ...
> How is a poor filesystem supposed to make that true? Also btrfs has this
> problem since it uses private_2 for its own purposes.
On Wed, Mar 10, 2021 at 04:54:49PM +, David Howells wrote:
> Add a function, unlock_page_private_2(), to unlock PG_private_2 analogous
> to that of PG_lock. Add a kerneldoc banner to that indicating the example
> usage case.
This isn't a problem with this patch per se, but I'm concerned
Add a function, unlock_page_private_2(), to unlock PG_private_2 analogous
to that of PG_lock. Add a kerneldoc banner to that indicating the example
usage case.
A wrapper will need to be placed in the netfs header in the patch that adds
that.
[This implements a suggestion by Linus[1] to not mix
15 matches
Mail list logo
