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. ;) -- 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
