| Message ID | 4a4634330908171331n28e7d9fcif14b606f935271f8@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 17 Aug 2009 15:31:43 -0500 Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > Jeff, > > Hope this works. > Thanks for sending it. Comments inline below: > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 6084d63..9ad3258 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -349,6 +349,7 @@ struct cifsFileInfo { > struct mutex lock_mutex; > struct list_head llist; /* list of byte range locks we have. */ > bool closePend:1; /* file is marked to close */ > + bool closed:1; /* file is closed */ > bool invalidHandle:1; /* file closed via session abend */ > bool messageMode:1; /* for pipes: message vs byte mode */ > atomic_t wrtPending; /* handle in use - defer close */ > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index c34b7f8..f1ae25c 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -698,7 +698,10 @@ int cifs_close(struct inode *inode, struct file *file) > msleep(timeout); > timeout *= 8; > } > - kfree(file->private_data); > + if (atomic_read(&pSMBFile->wrtPending) == 0) > + kfree(file->private_data); > + else > + pSMBFile->closed = true; This is racy. It's possible that wrtPending will change just after you check it but before you kfree or set closed to true. > file->private_data = NULL; > } else > rc = -EBADF; > @@ -1293,7 +1296,10 @@ refind_writable: > else { /* start over in case this was deleted */ > /* since the list could be modified */ > read_lock(&GlobalSMBSeslock); > - atomic_dec(&open_file->wrtPending); > + if (atomic_dec_and_test( > + &open_file->wrtPending) > + && open_file->closed) > + kfree(open_file); > goto refind_writable; > } > } > @@ -1309,10 +1315,12 @@ refind_writable: > read_lock(&GlobalSMBSeslock); > /* can not use this handle, no write > pending on this one after all */ > - atomic_dec(&open_file->wrtPending); > - > - if (open_file->closePend) /* list could have changed */ > + if (atomic_dec_and_test(&open_file->wrtPending) > + && open_file->closed) { > + kfree(open_file); > goto refind_writable; > + } else if (open_file->closePend) /* list could */ > + goto refind_writable; /* have changed */ > /* else we simply continue to the next entry. Thus > we do not loop on reopen errors. If we > can not reopen the file, for example if we > @@ -1373,7 +1381,9 @@ static int cifs_partialpagewrite(struct page > *page, unsigned from, unsigned to) > if (open_file) { > bytes_written = cifs_write(open_file->pfile, write_data, > to-from, &offset); > - atomic_dec(&open_file->wrtPending); > + if (atomic_dec_and_test(&open_file->wrtPending) > + && open_file->closed) > + kfree(open_file); > /* Does mm or vfs already set times? */ > inode->i_atime = inode->i_mtime = current_fs_time(inode->i_sb); > if ((bytes_written > 0) && (offset)) > @@ -1562,7 +1572,9 @@ retry: > bytes_to_write, offset, > &bytes_written, iov, n_iov, > long_op); > - atomic_dec(&open_file->wrtPending); > + if (atomic_dec_and_test(&open_file->wrtPending) > + && open_file->closed) > + kfree(open_file); > cifs_update_eof(cifsi, offset, bytes_written); > > if (rc || bytes_written < bytes_to_write) { > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index 82d8383..3b4c27d 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -799,8 +799,11 @@ set_via_filehandle: > > if (open_file == NULL) > CIFSSMBClose(xid, pTcon, netfid); > - else > - atomic_dec(&open_file->wrtPending); > + else { > + if (atomic_dec_and_test(&open_file->wrtPending) > + && open_file->closed) > + kfree(open_file); > + } > out: > return rc; > } > @@ -1635,7 +1638,9 @@ cifs_set_file_size(struct inode *inode, struct > iattr *attrs, > __u32 npid = open_file->pid; > rc = CIFSSMBSetFileSize(xid, pTcon, attrs->ia_size, nfid, > npid, false); > - atomic_dec(&open_file->wrtPending); > + if (atomic_dec_and_test(&open_file->wrtPending) > + && open_file->closed) > + kfree(open_file); > cFYI(1, ("SetFSize for attrs rc = %d", rc)); > if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) { > unsigned int bytes_written; > @@ -1790,7 +1795,9 @@ cifs_setattr_unix(struct dentry *direntry, > struct iattr *attrs) > u16 nfid = open_file->netfid; > u32 npid = open_file->pid; > rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid); > - atomic_dec(&open_file->wrtPending); > + if (atomic_dec_and_test(&open_file->wrtPending) > + && open_file->closed) > + kfree(open_file); > } else { > rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args, > cifs_sb->local_nls, > > > signed-off by: Shirish Pargaonkar <shirishpargaonkar@gmail.com> > I'm less than thrilled with this patch. This looks like like it's layering more complexity onto a codepath that is already far too complex for what it does. Is it not possible to just use regular old refcounting for the open filehandles? i.e. have each user of the filehandle take a reference, and the last one to put it does the actual close. That seems like a much better approach to me than all of this crazy flag business.
On Tue, Aug 18, 2009 at 6:15 AM, Jeff Layton<jlayton@redhat.com> wrote: > On Mon, 17 Aug 2009 15:31:43 -0500 > Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > >> Jeff, >> >> Hope this works. >> > > Thanks for sending it. Comments inline below: > >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h >> index 6084d63..9ad3258 100644 >> --- a/fs/cifs/cifsglob.h >> +++ b/fs/cifs/cifsglob.h >> @@ -349,6 +349,7 @@ struct cifsFileInfo { >> struct mutex lock_mutex; >> struct list_head llist; /* list of byte range locks we have. */ >> bool closePend:1; /* file is marked to close */ >> + bool closed:1; /* file is closed */ >> bool invalidHandle:1; /* file closed via session abend */ >> bool messageMode:1; /* for pipes: message vs byte mode */ >> atomic_t wrtPending; /* handle in use - defer close */ >> diff --git a/fs/cifs/file.c b/fs/cifs/file.c >> index c34b7f8..f1ae25c 100644 >> --- a/fs/cifs/file.c >> +++ b/fs/cifs/file.c >> @@ -698,7 +698,10 @@ int cifs_close(struct inode *inode, struct file *file) >> msleep(timeout); >> timeout *= 8; >> } >> - kfree(file->private_data); >> + if (atomic_read(&pSMBFile->wrtPending) == 0) >> + kfree(file->private_data); >> + else >> + pSMBFile->closed = true; > > This is racy. It's possible that wrtPending will change just after you > check it but before you kfree or set closed to true. May be we could do handle wrtPending within lock like GlobalSMBSeslock and perhaps get rid of msleep while waiting for wrtPending to become 0. (add some code, loose some code) > >> file->private_data = NULL; >> } else >> rc = -EBADF; >> @@ -1293,7 +1296,10 @@ refind_writable: >> else { /* start over in case this was deleted */ >> /* since the list could be modified */ >> read_lock(&GlobalSMBSeslock); >> - atomic_dec(&open_file->wrtPending); >> + if (atomic_dec_and_test( >> + &open_file->wrtPending) >> + && open_file->closed) >> + kfree(open_file); >> goto refind_writable; >> } >> } >> @@ -1309,10 +1315,12 @@ refind_writable: >> read_lock(&GlobalSMBSeslock); >> /* can not use this handle, no write >> pending on this one after all */ >> - atomic_dec(&open_file->wrtPending); >> - >> - if (open_file->closePend) /* list could have changed */ >> + if (atomic_dec_and_test(&open_file->wrtPending) >> + && open_file->closed) { >> + kfree(open_file); >> goto refind_writable; >> + } else if (open_file->closePend) /* list could */ >> + goto refind_writable; /* have changed */ >> /* else we simply continue to the next entry. Thus >> we do not loop on reopen errors. If we >> can not reopen the file, for example if we >> @@ -1373,7 +1381,9 @@ static int cifs_partialpagewrite(struct page >> *page, unsigned from, unsigned to) >> if (open_file) { >> bytes_written = cifs_write(open_file->pfile, write_data, >> to-from, &offset); >> - atomic_dec(&open_file->wrtPending); >> + if (atomic_dec_and_test(&open_file->wrtPending) >> + && open_file->closed) >> + kfree(open_file); >> /* Does mm or vfs already set times? */ >> inode->i_atime = inode->i_mtime = current_fs_time(inode->i_sb); >> if ((bytes_written > 0) && (offset)) >> @@ -1562,7 +1572,9 @@ retry: >> bytes_to_write, offset, >> &bytes_written, iov, n_iov, >> long_op); >> - atomic_dec(&open_file->wrtPending); >> + if (atomic_dec_and_test(&open_file->wrtPending) >> + && open_file->closed) >> + kfree(open_file); >> cifs_update_eof(cifsi, offset, bytes_written); >> >> if (rc || bytes_written < bytes_to_write) { >> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c >> index 82d8383..3b4c27d 100644 >> --- a/fs/cifs/inode.c >> +++ b/fs/cifs/inode.c >> @@ -799,8 +799,11 @@ set_via_filehandle: >> >> if (open_file == NULL) >> CIFSSMBClose(xid, pTcon, netfid); >> - else >> - atomic_dec(&open_file->wrtPending); >> + else { >> + if (atomic_dec_and_test(&open_file->wrtPending) >> + && open_file->closed) >> + kfree(open_file); >> + } >> out: >> return rc; >> } >> @@ -1635,7 +1638,9 @@ cifs_set_file_size(struct inode *inode, struct >> iattr *attrs, >> __u32 npid = open_file->pid; >> rc = CIFSSMBSetFileSize(xid, pTcon, attrs->ia_size, nfid, >> npid, false); >> - atomic_dec(&open_file->wrtPending); >> + if (atomic_dec_and_test(&open_file->wrtPending) >> + && open_file->closed) >> + kfree(open_file); >> cFYI(1, ("SetFSize for attrs rc = %d", rc)); >> if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) { >> unsigned int bytes_written; >> @@ -1790,7 +1795,9 @@ cifs_setattr_unix(struct dentry *direntry, >> struct iattr *attrs) >> u16 nfid = open_file->netfid; >> u32 npid = open_file->pid; >> rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid); >> - atomic_dec(&open_file->wrtPending); >> + if (atomic_dec_and_test(&open_file->wrtPending) >> + && open_file->closed) >> + kfree(open_file); >> } else { >> rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args, >> cifs_sb->local_nls, >> >> >> signed-off by: Shirish Pargaonkar <shirishpargaonkar@gmail.com> >> > > I'm less than thrilled with this patch. This looks like like it's > layering more complexity onto a codepath that is already far too > complex for what it does. > > Is it not possible to just use regular old refcounting for the open > filehandles? i.e. have each user of the filehandle take a reference, > and the last one to put it does the actual close. That seems like a > much better approach to me than all of this crazy flag business. > I think this needs, some re-designing the code right? > -- > Jeff Layton <jlayton@redhat.com> >
On Tue, 18 Aug 2009 10:23:09 -0500 Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > On Tue, Aug 18, 2009 at 6:15 AM, Jeff Layton<jlayton@redhat.com> wrote: > > On Mon, 17 Aug 2009 15:31:43 -0500 > > Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > > > >> Jeff, > >> > >> Hope this works. > >> > > > > Thanks for sending it. Comments inline below: > > > >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > >> index 6084d63..9ad3258 100644 > >> --- a/fs/cifs/cifsglob.h > >> +++ b/fs/cifs/cifsglob.h > >> @@ -349,6 +349,7 @@ struct cifsFileInfo { > >> struct mutex lock_mutex; > >> struct list_head llist; /* list of byte range locks we have. */ > >> bool closePend:1; /* file is marked to close */ > >> + bool closed:1; /* file is closed */ > >> bool invalidHandle:1; /* file closed via session abend */ > >> bool messageMode:1; /* for pipes: message vs byte mode */ > >> atomic_t wrtPending; /* handle in use - defer close */ > >> diff --git a/fs/cifs/file.c b/fs/cifs/file.c > >> index c34b7f8..f1ae25c 100644 > >> --- a/fs/cifs/file.c > >> +++ b/fs/cifs/file.c > >> @@ -698,7 +698,10 @@ int cifs_close(struct inode *inode, struct file *file) > >> msleep(timeout); > >> timeout *= 8; > >> } > >> - kfree(file->private_data); > >> + if (atomic_read(&pSMBFile->wrtPending) == 0) > >> + kfree(file->private_data); > >> + else > >> + pSMBFile->closed = true; > > > > This is racy. It's possible that wrtPending will change just after you > > check it but before you kfree or set closed to true. > > May be we could do handle wrtPending within lock like GlobalSMBSeslock > and perhaps get rid of msleep while waiting for wrtPending to become 0. > (add some code, loose some code) > My vote would be no. It's time to stop overloading existing locks like this. The more I look at the open filehandle code, the more I'm convinced that it's just a fundamentally poor design. If you want to go that route, then I think you need to first step back and document the locking rules for the open filehandle code. What data structures does the GlobalSMBSesLock protect? > > > >> file->private_data = NULL; > >> } else > >> rc = -EBADF; > >> @@ -1293,7 +1296,10 @@ refind_writable: > >> else { /* start over in case this was deleted */ > >> /* since the list could be modified */ > >> read_lock(&GlobalSMBSeslock); > >> - atomic_dec(&open_file->wrtPending); > >> + if (atomic_dec_and_test( > >> + &open_file->wrtPending) > >> + && open_file->closed) > >> + kfree(open_file); > >> goto refind_writable; > >> } > >> } > >> @@ -1309,10 +1315,12 @@ refind_writable: > >> read_lock(&GlobalSMBSeslock); > >> /* can not use this handle, no write > >> pending on this one after all */ > >> - atomic_dec(&open_file->wrtPending); > >> - > >> - if (open_file->closePend) /* list could have changed */ > >> + if (atomic_dec_and_test(&open_file->wrtPending) > >> + && open_file->closed) { > >> + kfree(open_file); > >> goto refind_writable; > >> + } else if (open_file->closePend) /* list could */ > >> + goto refind_writable; /* have changed */ > >> /* else we simply continue to the next entry. Thus > >> we do not loop on reopen errors. If we > >> can not reopen the file, for example if we > >> @@ -1373,7 +1381,9 @@ static int cifs_partialpagewrite(struct page > >> *page, unsigned from, unsigned to) > >> if (open_file) { > >> bytes_written = cifs_write(open_file->pfile, write_data, > >> to-from, &offset); > >> - atomic_dec(&open_file->wrtPending); > >> + if (atomic_dec_and_test(&open_file->wrtPending) > >> + && open_file->closed) > >> + kfree(open_file); > >> /* Does mm or vfs already set times? */ > >> inode->i_atime = inode->i_mtime = current_fs_time(inode->i_sb); > >> if ((bytes_written > 0) && (offset)) > >> @@ -1562,7 +1572,9 @@ retry: > >> bytes_to_write, offset, > >> &bytes_written, iov, n_iov, > >> long_op); > >> - atomic_dec(&open_file->wrtPending); > >> + if (atomic_dec_and_test(&open_file->wrtPending) > >> + && open_file->closed) > >> + kfree(open_file); > >> cifs_update_eof(cifsi, offset, bytes_written); > >> > >> if (rc || bytes_written < bytes_to_write) { > >> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > >> index 82d8383..3b4c27d 100644 > >> --- a/fs/cifs/inode.c > >> +++ b/fs/cifs/inode.c > >> @@ -799,8 +799,11 @@ set_via_filehandle: > >> > >> if (open_file == NULL) > >> CIFSSMBClose(xid, pTcon, netfid); > >> - else > >> - atomic_dec(&open_file->wrtPending); > >> + else { > >> + if (atomic_dec_and_test(&open_file->wrtPending) > >> + && open_file->closed) > >> + kfree(open_file); > >> + } > >> out: > >> return rc; > >> } > >> @@ -1635,7 +1638,9 @@ cifs_set_file_size(struct inode *inode, struct > >> iattr *attrs, > >> __u32 npid = open_file->pid; > >> rc = CIFSSMBSetFileSize(xid, pTcon, attrs->ia_size, nfid, > >> npid, false); > >> - atomic_dec(&open_file->wrtPending); > >> + if (atomic_dec_and_test(&open_file->wrtPending) > >> + && open_file->closed) > >> + kfree(open_file); > >> cFYI(1, ("SetFSize for attrs rc = %d", rc)); > >> if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) { > >> unsigned int bytes_written; > >> @@ -1790,7 +1795,9 @@ cifs_setattr_unix(struct dentry *direntry, > >> struct iattr *attrs) > >> u16 nfid = open_file->netfid; > >> u32 npid = open_file->pid; > >> rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid); > >> - atomic_dec(&open_file->wrtPending); > >> + if (atomic_dec_and_test(&open_file->wrtPending) > >> + && open_file->closed) > >> + kfree(open_file); > >> } else { > >> rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args, > >> cifs_sb->local_nls, > >> > >> > >> signed-off by: Shirish Pargaonkar <shirishpargaonkar@gmail.com> > >> > > > > I'm less than thrilled with this patch. This looks like like it's > > layering more complexity onto a codepath that is already far too > > complex for what it does. > > > > Is it not possible to just use regular old refcounting for the open > > filehandles? i.e. have each user of the filehandle take a reference, > > and the last one to put it does the actual close. That seems like a > > much better approach to me than all of this crazy flag business. > > > > I think this needs, some re-designing the code right? > My vote would be "yes". Redesign and simplify the code rather than adding in new hacks to work around the flaws in the existing design. Redesigning it with actual refcounting (and not this half-assed wrtPending stuff) seems like a much better approach.
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 6084d63..9ad3258 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -349,6 +349,7 @@ struct cifsFileInfo { struct mutex lock_mutex; struct list_head llist; /* list of byte range locks we have. */ bool closePend:1; /* file is marked to close */ + bool closed:1; /* file is closed */ bool invalidHandle:1; /* file closed via session abend */ bool messageMode:1; /* for pipes: message vs byte mode */ atomic_t wrtPending; /* handle in use - defer close */ diff --git a/fs/cifs/file.c b/fs/cifs/file.c index c34b7f8..f1ae25c 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -698,7 +698,10 @@ int cifs_close(struct inode *inode, struct file *file) msleep(timeout); timeout *= 8; } - kfree(file->private_data); + if (atomic_read(&pSMBFile->wrtPending) == 0) + kfree(file->private_data); + else + pSMBFile->closed = true; file->private_data = NULL; } else rc = -EBADF; @@ -1293,7 +1296,10 @@ refind_writable: else { /* start over in case this was deleted */ /* since the list could be modified */ read_lock(&GlobalSMBSeslock); - atomic_dec(&open_file->wrtPending); + if (atomic_dec_and_test( + &open_file->wrtPending) + && open_file->closed) + kfree(open_file); goto refind_writable; } } @@ -1309,10 +1315,12 @@ refind_writable: read_lock(&GlobalSMBSeslock); /* can not use this handle, no write pending on this one after all */ - atomic_dec(&open_file->wrtPending); - - if (open_file->closePend) /* list could have changed */ + if (atomic_dec_and_test(&open_file->wrtPending) + && open_file->closed) { + kfree(open_file); goto refind_writable; + } else if (open_file->closePend) /* list could */ + goto refind_writable; /* have changed */ /* else we simply continue to the next entry. Thus we do not loop on reopen errors. If we can not reopen the file, for example if we @@ -1373,7 +1381,9 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to) if (open_file) { bytes_written = cifs_write(open_file->pfile, write_data, to-from, &offset); - atomic_dec(&open_file->wrtPending); + if (atomic_dec_and_test(&open_file->wrtPending) + && open_file->closed) + kfree(open_file); /* Does mm or vfs already set times? */ inode->i_atime = inode->i_mtime = current_fs_time(inode->i_sb); if ((bytes_written > 0) && (offset)) @@ -1562,7 +1572,9 @@ retry: bytes_to_write, offset, &bytes_written, iov, n_iov, long_op); - atomic_dec(&open_file->wrtPending); + if (atomic_dec_and_test(&open_file->wrtPending) + && open_file->closed) + kfree(open_file); cifs_update_eof(cifsi, offset, bytes_written); if (rc || bytes_written < bytes_to_write) { diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 82d8383..3b4c27d 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -799,8 +799,11 @@ set_via_filehandle: if (open_file == NULL) CIFSSMBClose(xid, pTcon, netfid); - else - atomic_dec(&open_file->wrtPending); + else { + if (atomic_dec_and_test(&open_file->wrtPending) + && open_file->closed) + kfree(open_file); + } out: return rc; } @@ -1635,7 +1638,9 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, __u32 npid = open_file->pid; rc = CIFSSMBSetFileSize(xid, pTcon, attrs->ia_size, nfid, npid, false); - atomic_dec(&open_file->wrtPending); + if (atomic_dec_and_test(&open_file->wrtPending) + && open_file->closed) + kfree(open_file); cFYI(1, ("SetFSize for attrs rc = %d", rc)); if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) {
Jeff, Hope this works. unsigned int bytes_written; @@ -1790,7 +1795,9 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) u16 nfid = open_file->netfid; u32 npid = open_file->pid; rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid); - atomic_dec(&open_file->wrtPending); + if (atomic_dec_and_test(&open_file->wrtPending) + && open_file->closed) + kfree(open_file); } else { rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args, cifs_sb->local_nls, signed-off by: Shirish Pargaonkar <shirishpargaonkar@gmail.com> On Mon, Aug 17, 2009 at 2:50 PM, Jeff Layton<jlayton@redhat.com> wrote: > On Sun, 16 Aug 2009 11:38:57 -0500 > Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > >> This patch prevents a slab corruption like this. Â During heavy stress, >> it is possible that >> cifs_close will free up cifsFileInfo while due to delayed writes, >> wrtPending of that >> cifsFileInfo gets updated (decremented), cifsFileInfo either freed or >> freed and allocated to another process. >> >> >> Slab corruption: start=ffff8101e28e3818, len=256 >> Redzone: 0x5a2cf071/0x5a2cf071. >> Last user: [<ffffffff88276ec4>](cifs_close+0x224/0x2c2 [cifs]) >> 060: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6a 6b 6b 6b >> Prev obj: start=ffff8101e28e3700, len=256 >> Redzone: 0x170fc2a5/0x170fc2a5. >> Last user: [<ffffffff882784fa>](cifs_open+0x348/0x6d9 [cifs]) >> 000: b8 d3 44 e2 01 81 ff ff a0 e2 c7 e1 01 81 ff ff >> 010: 68 f5 fa dc 01 81 ff ff 10 47 d4 dd 01 81 ff ff >> Next obj: start=ffff8101e28e3930, len=256 >> Redzone: 0x170fc2a5/0x170fc2a5. >> Last user: [<ffffffff882784fa>](cifs_open+0x348/0x6d9 [cifs]) >> 000: 18 b8 2a e3 01 81 ff ff b8 a3 5a e0 01 81 ff ff >> 010: 68 f5 36 65 02 81 ff ff 40 99 a1 dc 01 81 ff ff > > Can you resend this patch with it inlined into the email? I can't > reasonably comment on it with it sent as a binary attachment. > > Thanks, > -- > Jeff Layton <jlayton@redhat.com> >