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.
Mikulas
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
