On Thu, Sep 12 2013 at 7:27pm -0400,
Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
>
> On Thu, 12 Sep 2013, Mike Snitzer wrote:
>
> > On Thu, Sep 12 2013 at 6:45pm -0400,
> > Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> >
> > > I think this is too complicated.
> > >
> > > dm-bufio uses similar approach like this patch - but in dm-bufio the code
> > > path is performance critical, we don't want to recalculate memory size
> > > with each i/o request.
> > >
> > > Here it is not performance critical, so I'd drop the mutex, drop the
> > > latch, drop the function __reserved_request_based_ios_refresh and add only
> > > these lines:
> > >
> > > pool_size = ACCESS_ONCE(reserved_rq_based_ios);
> > > if (!pool_size) pool_size = RESERVED_REQUEST_BASED_IOS;
> >
> > Could be I'm missing something but isn't the locking about correctness
> > rather than performance?
>
> If the user changes the value while you are allocating the mempool, you
> can allocate the mempool with the old or new value, it doesn't matter.
> There is nothing to lock against.
>
> > Also, using the latch enables the dm_get_reserved_rq_based_ios()
> > interface that is used in patch 5.
> >
> > Mike
>
> You can just define dm_get_reserved_rq_based_ios as this:
> unsigned dm_get_reserved_rq_based_ios(void)
> {
> unsigned pool_size = ACCESS_ONCE(reserved_rq_based_ios);
> if (!pool_size) pool_size = RESERVED_REQUEST_BASED_IOS;
> return pool_size;
> }
> ... and call it when you need the pool size.
>
> The purpose of the latch in dm-bufio is to avoid memory size recalculation
> with each buffer allocation. Here, there is nothing to recalculate and the
> code path is not performance-critical.
OK, I'll revise the patch.
Thanks,
Mike
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
