On Tue, 23 Jul 2013 11:46:51 +0400, Vyacheslav Dubeyko wrote: > Hi Ryusuke, > > On Tue, 2013-07-23 at 03:24 +0900, Ryusuke Konishi wrote: > > [snip] >> >> Thank you for following the issue. I reviewed the code around bio. >> >> In conclusion, Dan Carpenter's patch looks correct because >> nilfs_segbuf_submit_bio() does not increment the number of flying bio >> (segbuf->sb_nbio) for EOPNOTSUPP/BIO_EOPNOTSUPP case. >> > > I worry that nilfs_end_bio_write() is called asynchronously. And, as I > understand, the BIO_EOPNOTSUPP flag is set during nilfs_end_bio_write() > call. It means for me that sometimes segbuf->sb_nbio will be not > incremented in nilfs_segbuf_submit_bio() but sometimes this field can be > incremented and for BIO_EOPNOTSUPP case. > >> If nilfs_end_bio_write() function reaches the complete() call for the >> EOPNOTSUPP/BIO_EOPNOTSUPP case (as the current implementation), the >> number of complete() calls and that of wait_for_complete() will not >> balance. >> > > I think that it is dangerous to return without complete() call because > of asynchronous nature of nilfs_end_bio_write() call. > > What do you think? nilfs_end_bio_write() may be called either synchronously or asynchronously as bio->bi_end_io(). It may be called when submit_bio() fails or when bio is terminated asynchronously. Yes, the current nilfs_end_bio_write() implementation is confusing and dangerous for the EOPNOTSUPP case. I think nilfs_end_bio_write() should be modified so that segbuf->sb_nbio is incremented in either case. Thanks, Ryusuke Konishi -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
