Re: [PATCH] CIFS: Use invalidate_inode_pages2 instead of invalidate_remote_inode (try #4)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 22 Mar 2011 15:01:11 +0300
Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:

> 2011/3/22 Jeff Layton <jlayton@xxxxxxxxxx>:
> > On Wed, 16 Mar 2011 01:55:32 +0300
> > Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
> >
> >> Use invalidate_inode_pages2 that don't leave pages even if shrink_page_list()
> >> has a temp ref on them. It prevents a data coherency problem when
> >> cifs_invalidate_mapping didn't invalidate pages but the client thinks that a data
> >> from the cache is uptodate according to an oplock level (exclusive or II).
> >>
> >> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
> >> ---
> >>  fs/cifs/inode.c |   10 ++++++++--
> >>  1 files changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> >> index 41e5651..adb6324 100644
> >> --- a/fs/cifs/inode.c
> >> +++ b/fs/cifs/inode.c
> >> @@ -1691,12 +1691,18 @@ cifs_invalidate_mapping(struct inode *inode)
> >>
> >>       cifs_i->invalid_mapping = false;
> >>
> >> -     /* write back any cached data */
> >>       if (inode->i_mapping && inode->i_mapping->nrpages != 0) {
> >> +             /* write back any cached data */
> >>               rc = filemap_write_and_wait(inode->i_mapping);
> >>               mapping_set_error(inode->i_mapping, rc);
> >> +             rc = invalidate_inode_pages2(inode->i_mapping);
> >> +             if (rc) {
> >> +                     cERROR(1, "%s: could not invalidate inode %p", __func__,
> >> +                            inode);
> >> +                     cifs_i->invalid_mapping = true;
> >> +             }
> >>       }
> >> -     invalidate_remote_inode(inode);
> >> +
> >>       cifs_fscache_reset_inode_cookie(inode);
> >>  }
> >>
> >
> > Hi Pavel,
> >
> > I noticed that Steve has merged this patch, but it doesn't seem like
> > you ever made the change to have EBUSY percolate up to userspace. Are
> > you still planning to fix that?
> >
> 
> Steve NACK'ed this and we came to agreement to mark a mapping as
> invalid in this case for now and let the client try invalidate it
> again when it needs. Of course, it can bring problems with a data
> coherency if you need to use a cache very strictly.
> 

Ahh, I must have missed the discussion around that NAK, and I don't see
any record of it in the list archives. Steve, can you clarify why you
didn't think that approach was acceptible?

-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux