On Thu, Jul 13, 2017 at 09:45:19AM +0200, Christoph Hellwig wrote:
> >  static void bio_free(struct bio *bio)
> >  {
> >     struct bio_set *bs = bio->bi_pool;
> >     void *p;
> >  
> > -   bio_uninit(bio);
> > +   bio_disassociate_task(bio);
> 
> As said in the last mail I think there is no point in having this call..

I'm hesitant to do this. bio_associate_blkcg/bio_associate_current can be
called in any time for a bio, so we not just attach cgroup info to info in bio
submit (maybe the bio_associate_blkcg/bio_associate_current callers do sumbit
always, but I didn't audit yet). The other reason is I'd like
blk_throtl_bio_endio is only called once for the whole bio not for splitted
bio, so this depends on bio_free to free cgroup info for chained bio which
always does bio_put.

> > @@ -294,7 +289,7 @@ void bio_reset(struct bio *bio)
> >  {
> >     unsigned long flags = bio->bi_flags & (~0UL << BIO_RESET_BITS);
> >  
> > -   bio_uninit(bio);
> > +   bio_disassociate_task(bio);
> 
> .. and this one.  And I suspect it would make sense to have all these
> changes in a single patch.

This one is likely ok, but again I didn't audit yet. I'm ok these changes are
in a single patch.

Thanks,
Shaohua

Reply via email to