[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [dm-devel] [PATCH 2/7] dm: add reserved_rq_based_ios module parameter




On Thu, 12 Sep 2013, Mike Snitzer wrote:

> On Thu, Sep 12 2013 at  6:45pm -0400,
> Mikulas Patocka <mpatocka redhat com> 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]