On Mon, 4 Oct 2010 13:20:57 -0400 Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Mon, 4 Oct 2010 13:05:34 -0400 > Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > > On Sat, Sep 25, 2010 at 08:16:44AM -0400, Jeff Layton wrote: > > > Hmm...there is one problem with this scheme. __fput ignores the error > > > return from ->release. Only the errors from ->flush will be returned to > > > userspace. So if we only filemap_fdatawait in the ->release op, then we > > > have the potential to miss returning writeback-related errors on a > > > close call. > > > > > > On a side note, why does f_op->release return an int? Are there places > > > in the kernel besides __fput that call it? If not, maybe we should > > > consider changing it to a void function to make this more clear. > > > > If I remember correctly actually returning errors from ->release > > actually caused problems with various bits of userspace software. I'd > > need to check the commit logs for details. No idea why these problems > > don't appear for ->flush or why we've never bothered to change the > > ->release prototype. > > I asked Al about it, and he basically said that it had been an int > return for a long, long time. By the time we get to ->release it's far > too late to really be able to do anything about an error, so there's > really no benefit to keeping it an int return. > > The only obstacle is code churn. I also suspect that a lot of ->release > ops return an error, thinking it'll do something too. What Al suggested > for a first step was to add a WARN() or something whenever a ->release > op returns non-zero. I'm not looking to tackle that myself at the > moment however. ;) > For CIFS though, this means that we really do need to wait for writeback to complete in the ->flush operation. -- 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
