Re: New -udm?

Re: New -udm?

[goggin, edward]

On Mon, 11 Apr 2005 04:53:07 -0700
Mike Christie <mikenc@us.ibm.com>
> 
> Lars Marowsky-Bree wrote:
> > On 2005-04-11T02:27:11, Mike Christie <mikenc@us.ibm.com> wrote:
> > 
> > 
> >>what is wrong with what you have now where you utilize the 
> queue/path's 
> >>mempool by doing a blk_get_request with GFP_WAIT? 
> > 
> > 
> > ... what if it's trying to free memory by going to swap on 
> multipath,
> > and can't, because we're blocked on getting the request with
> > GFP_WAIT...?
> 
> GFP_WAIT does not casue IOs though. That is the difference between 
> waiing on GFP_KERNEL and GFP_WAIT I thought. GFP_KERNEL can 
> cause a page 
> write out which you wait on and then there is a problem since 
> it could 
> be to the same disk you are trying to recover. But if you are just 
> waiting for something to be returned to the mempool like with 
> GFP_WAIT + 
> blk_get_request you should be ok as long as the code below you 
> eventually give up their resources and frees the requests you are 
> waiting on?
>
 
A deterministic, fool-proof solution for this case must deal with
the possibility that in order to make progress, one cannot depend
that any memory resource which has previously been used will free
up -- because the freeing of that memory may be dependent on
making progress at this point.  Even using GFP_WAIT, it is possible
that all previously allocated bio (not sure about requests) mempool
resources needed are queued waiting for a multipath path to become
usable again.

I don't see a way around needing to use pre-allocated bio memory
which is reserved strictly for this purpose -- albeit it is possible
that a single bio could be reserved for making progress in serial
fashion across all multipaths which are in this state.

> > 
> > Your patch helps, because it means we need less memory.
> > 
> > But, ultimately, we ought to preallocate the requests 
> already when the
> > hw-handler is initialized for a map (because presumably at that time
> > we'll have enough memory, or can just fail the table 
> setup). From that
> > point on, our memory useage should not grow.
> >

Re: New -udm?

[Mike Christie]

goggin, edward wrote:
> On Mon, 11 Apr 2005 04:53:07 -0700
> Mike Christie <mikenc@us.ibm.com>
> 
>>Lars Marowsky-Bree wrote:
>>
>>>On 2005-04-11T02:27:11, Mike Christie <mikenc@us.ibm.com> wrote:
>>>
>>>
>>>
>>>>what is wrong with what you have now where you utilize the 
>>
>>queue/path's 
>>
>>>>mempool by doing a blk_get_request with GFP_WAIT? 
>>>
>>>
>>>... what if it's trying to free memory by going to swap on 
>>
>>multipath,
>>
>>>and can't, because we're blocked on getting the request with
>>>GFP_WAIT...?
>>
>>GFP_WAIT does not casue IOs though. That is the difference between 
>>waiing on GFP_KERNEL and GFP_WAIT I thought. GFP_KERNEL can 
>>cause a page 
>>write out which you wait on and then there is a problem since 
>>it could 
>>be to the same disk you are trying to recover. But if you are just 
>>waiting for something to be returned to the mempool like with 
>>GFP_WAIT + 
>>blk_get_request you should be ok as long as the code below you 
>>eventually give up their resources and frees the requests you are 
>>waiting on?
>>
> 
>  
> A deterministic, fool-proof solution for this case must deal with
> the possibility that in order to make progress, one cannot depend
> that any memory resource which has previously been used will free
> up -- because the freeing of that memory may be dependent on
> making progress at this point.  Even using GFP_WAIT, it is possible
> that all previously allocated bio (not sure about requests) mempool
> resources needed are queued waiting for a multipath path to become
> usable again.

For requests...
I thought the only way you can have a problem is if userspace is doing 
sg io directly to the paths, like the path testers, and the lower level 
drivers do not free resources (like if some evil scsi driver decides to 
block forever in its error handler while multiple sg io requests are 
queued for example). There is no one else above us so we do not have to 
worry about that case where people above dm call blk_get_request on the 
same queue, deplete the mempool and never free those requests so we 
block forever on some layer above us which cannot proceed becuase we cannot.

Using blk_get_request + GFP_WAIT allows dm to use the same block layer 
code path as __make_request for memory allcoation failures, so if there 
is a problem with that code itself (like needing to preallocate 
correctly or some other issue like that - I am not saying preallocation 
is a problem there) it should be solved for all of us there. It makes no 
sense to have a fool proof multipath but flakey block layer and if we 
can share code all the better.

> 
> I don't see a way around needing to use pre-allocated bio memory
> which is reserved strictly for this purpose -- albeit it is possible
> that a single bio could be reserved for making progress in serial
> fashion across all multipaths which are in this state.
> 
> 
>>>Your patch helps, because it means we need less memory.
>>>
>>>But, ultimately, we ought to preallocate the requests 
>>
>>already when the
>>
>>>hw-handler is initialized for a map (because presumably at that time
>>>we'll have enough memory, or can just fail the table 
>>
>>setup). From that
>>
>>>point on, our memory useage should not grow.
>>>
> 
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

Re: New -udm?

[Mike Christie]

Mike Christie wrote:
>>
>> I don't see a way around needing to use pre-allocated bio memory
>> which is reserved strictly for this purpose -- albeit it is possible
>> that a single bio could be reserved for making progress in serial
>> fashion across all multipaths which are in this state.
>>
>>

I agree for bios we are in trouble :)

Re: New -udm?

[Mike Christie]

Mike Christie wrote:
> goggin, edward wrote:
> 
>> On Mon, 11 Apr 2005 04:53:07 -0700
>> Mike Christie <mikenc@us.ibm.com>
>>
>>> Lars Marowsky-Bree wrote:
>>>
>>>> On 2005-04-11T02:27:11, Mike Christie <mikenc@us.ibm.com> wrote:
>>>>
>>>>
>>>>
>>>>> what is wrong with what you have now where you utilize the 
>>>
>>>
>>> queue/path's
>>>
>>>>> mempool by doing a blk_get_request with GFP_WAIT? 
>>>>
>>>>
>>>>
>>>> ... what if it's trying to free memory by going to swap on 
>>>
>>>
>>> multipath,
>>>
>>>> and can't, because we're blocked on getting the request with
>>>> GFP_WAIT...?
>>>
>>>
>>> GFP_WAIT does not casue IOs though. That is the difference between 
>>> waiing on GFP_KERNEL and GFP_WAIT I thought. GFP_KERNEL can cause a 
>>> page write out which you wait on and then there is a problem since it 
>>> could be to the same disk you are trying to recover. But if you are 
>>> just waiting for something to be returned to the mempool like with 
>>> GFP_WAIT + blk_get_request you should be ok as long as the code below 
>>> you eventually give up their resources and frees the requests you are 
>>> waiting on?
>>>
>>
>>  
>> A deterministic, fool-proof solution for this case must deal with
>> the possibility that in order to make progress, one cannot depend
>> that any memory resource which has previously been used will free
>> up -- because the freeing of that memory may be dependent on
>> making progress at this point.  Even using GFP_WAIT, it is possible
>> that all previously allocated bio (not sure about requests) mempool
>> resources needed are queued waiting for a multipath path to become
>> usable again.
> 
> 
> For requests...
> I thought the only way you can have a problem is if userspace is doing 
> sg io directly to the paths, like the path testers, and the lower level 
> drivers do not free resources (like if some evil scsi driver decides to 
> block forever in its error handler while multiple sg io requests are 
> queued for example). There is no one else above us so we do not have to 
> worry about that case where people above dm call blk_get_request on the 
> same queue, deplete the mempool and never free those requests so we 
> block forever on some layer above us which cannot proceed becuase we 
> cannot.
> 
> Using blk_get_request + GFP_WAIT allows dm to use the same block layer 
> code path as __make_request for memory allcoation failures, so if there 
> is a problem with that code itself (like needing to preallocate 
> correctly or some other issue like that - I am not saying preallocation 
> is a problem there) 

maybe starvation with multiple waiters though?

it should be solved for all of us there. It makes no
> sense to have a fool proof multipath but flakey block layer and if we 
> can share code all the better.
> 
>>
>> I don't see a way around needing to use pre-allocated bio memory
>> which is reserved strictly for this purpose -- albeit it is possible
>> that a single bio could be reserved for making progress in serial
>> fashion across all multipaths which are in this state.
>>
>>
>>>> Your patch helps, because it means we need less memory.
>>>>
>>>> But, ultimately, we ought to preallocate the requests 
>>>
>>>
>>> already when the
>>>
>>>> hw-handler is initialized for a map (because presumably at that time
>>>> we'll have enough memory, or can just fail the table 
>>>
>>>
>>> setup). From that
>>>
>>>> point on, our memory useage should not grow.
>>>>
>>
>>
>> -- 
>> dm-devel mailing list
>> dm-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/dm-devel
>>
> 
> 
> -- 
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

Re: New -udm?

[Mike Christie]

goggin, edward wrote:
> 
> I don't see a way around needing to use pre-allocated bio memory
> which is reserved strictly for this purpose -- albeit it is possible
> that a single bio could be reserved for making progress in serial
> fashion across all multipaths which are in this state.
> 
> 

Hey back on the subject of bio allocations. Is failback going to stay
in userspace? If so, the bio allocation there suffers from the same
problem. Would it work to have a fs_bio_set and block_bio_set so
the allocations coming into the block layer would use the fs set and
everything below (like block/scsi_ioctl or hw handlers) would use the
block layer one (today everyone uses the fs_bio_set which causes problems)?
If you stacked multipath on top of multipath I guess this would not work.
Or will you need per device bio sets?

For sg io there other allocations, so this just handles the bio itself.

New -udm?

[Lars Marowsky-Bree]

Hi Alasdair,

I'm resync'ing against -udm and would like to pull all recent fixes if
possible. Do you have anything more recent than the 2.6.11-rc3-udm2?


Sincerely,
    Lars Marowsky-Brée <lmb@suse.de>

-- 
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business

Re: New -udm?

[christophe varoqui]

On dim, 2005-04-10 at 21:29 +0200, Lars Marowsky-Bree wrote:
> Hi Alasdair,
> 
> I'm resync'ing against -udm and would like to pull all recent fixes if
> possible. Do you have anything more recent than the 2.6.11-rc3-udm2?
> 
> 
> Sincerely,
>     Lars Marowsky-Brée <lmb@suse.de>
> 
There is this hwhandler from Dave Olien, that needs auditing, but is a
must-have for StorageWorks and FSC FibreCat users.

http://christophe.varoqui.free.fr/multipath-tools/dm-scsi-start.patch

Would be good to have in an enterprise distro ...

Regards,
-- 
christophe varoqui <christophe.varoqui@free.fr>

Re: New -udm?

[Dave Olien]

Sadly, that implementation uses a depcreated ioctl.
I'll re-write it this week in the same manner the dm-emc.c
is coded.  Target to have it in the mail Friday.

On Sun, Apr 10, 2005 at 10:25:17PM +0200, christophe varoqui wrote:
> On dim, 2005-04-10 at 21:29 +0200, Lars Marowsky-Bree wrote:
> > Hi Alasdair,
> > 
> > I'm resync'ing against -udm and would like to pull all recent fixes if
> > possible. Do you have anything more recent than the 2.6.11-rc3-udm2?
> > 
> > 
> > Sincerely,
> >     Lars Marowsky-Brée <lmb@suse.de>
> > 
> There is this hwhandler from Dave Olien, that needs auditing, but is a
> must-have for StorageWorks and FSC FibreCat users.
> 
> http://christophe.varoqui.free.fr/multipath-tools/dm-scsi-start.patch
> 
> Would be good to have in an enterprise distro ...
> 
> Regards,
> -- 
> christophe varoqui <christophe.varoqui@free.fr>
> 
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: New -udm?

[Mike Christie]

christophe varoqui wrote:
> On dim, 2005-04-10 at 21:29 +0200, Lars Marowsky-Bree wrote:
> 
>>Hi Alasdair,
>>
>>I'm resync'ing against -udm and would like to pull all recent fixes if
>>possible. Do you have anything more recent than the 2.6.11-rc3-udm2?
>>
>>
>>Sincerely,
>>    Lars Marowsky-Brée <lmb@suse.de>
>>
> There is this hwhandler from Dave Olien, that needs auditing, but is a
> must-have for StorageWorks and FSC FibreCat users.
> 
> http://christophe.varoqui.free.fr/multipath-tools/dm-scsi-start.patch
> 
> Would be good to have in an enterprise distro ...
> 
> Regards,



+static void sstart_pg_init(struct hw_handler *hwh, unsigned bypassed,
+			struct path *path)
+{
+	struct block_device *bdev = path->dev->bdev;
+	struct gendisk *disk = bdev->bd_disk;
+	struct scsi_device *sdev = scsi_disk(disk)->device;
+	int result;
+
+	result = scsi_ioctl(sdev, SCSI_IOCTL_START_UNIT, NULL);



is scsi_ioctl depreciated for this use? Besides that I think it 
allocates memory with GFP_KERNEL which I do not think you want here, and 
you will want some way to gaurantee you can make forward progress.


+	dm_pg_init_complete(path, result?MP_FAIL_PATH:0);
+}
+

Re: New -udm?

[Dave Olien]

You're correct.  I'll rewrite it on Thursday this week.
I'll use the same methods Lars used in the dm-emc.c


On Sun, Apr 10, 2005 at 04:36:53PM -0700, Mike Christie wrote:
> christophe varoqui wrote:
> >On dim, 2005-04-10 at 21:29 +0200, Lars Marowsky-Bree wrote:
> >
> >>Hi Alasdair,
> >>
> >>I'm resync'ing against -udm and would like to pull all recent fixes if
> >>possible. Do you have anything more recent than the 2.6.11-rc3-udm2?
> >>
> >>
> >>Sincerely,
> >>   Lars Marowsky-Brée <lmb@suse.de>
> >>
> >There is this hwhandler from Dave Olien, that needs auditing, but is a
> >must-have for StorageWorks and FSC FibreCat users.
> >
> >http://christophe.varoqui.free.fr/multipath-tools/dm-scsi-start.patch
> >
> >Would be good to have in an enterprise distro ...
> >
> >Regards,
> 
> 
> 
> +static void sstart_pg_init(struct hw_handler *hwh, unsigned bypassed,
> +			struct path *path)
> +{
> +	struct block_device *bdev = path->dev->bdev;
> +	struct gendisk *disk = bdev->bd_disk;
> +	struct scsi_device *sdev = scsi_disk(disk)->device;
> +	int result;
> +
> +	result = scsi_ioctl(sdev, SCSI_IOCTL_START_UNIT, NULL);
> 
> 
> 
> is scsi_ioctl depreciated for this use? Besides that I think it 
> allocates memory with GFP_KERNEL which I do not think you want here, and 
> you will want some way to gaurantee you can make forward progress.
> 
> 
> +	dm_pg_init_complete(path, result?MP_FAIL_PATH:0);
> +}
> +
> 
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: New -udm?

[Lars Marowsky-Bree]

On 2005-04-10T18:14:44, Dave Olien <dmo@osdl.org> wrote:

> You're correct.  I'll rewrite it on Thursday this week.
> I'll use the same methods Lars used in the dm-emc.c

Note that dm-emc.c also would need to pre-allocate it's requests, but
doesn't right now :/

Pre-allocating the requests sucks: Either we pre-allocate for _every_
path we might potentially need to send the request down on, or fix up
the request for the path we sent it down on (which would require us to
use internal knowledge about the req structs we're not supposed to
have).

Good solutions solicited ;-)


Sincerely,
    Lars Marowsky-Brée <lmb@suse.de>

-- 
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business

Re: New -udm?

[Mike Christie]

Lars Marowsky-Bree wrote:
> On 2005-04-10T18:14:44, Dave Olien <dmo@osdl.org> wrote:
> 
> 
>>You're correct.  I'll rewrite it on Thursday this week.
>>I'll use the same methods Lars used in the dm-emc.c
> 
> 
> Note that dm-emc.c also would need to pre-allocate it's requests, but
> doesn't right now :/
> 
> Pre-allocating the requests sucks: Either we pre-allocate for _every_
> path we might potentially need to send the request down on, or fix up
> the request for the path we sent it down on (which would require us to
> use internal knowledge about the req structs we're not supposed to
> have).

what is wrong with what you have now where you utilize the queue/path's 
mempool by doing a blk_get_request with GFP_WAIT? Is that "fix up the 
request..." comment meaning that you do not like to access the request 
structure for that code, or was it meaning that you have one request 
that is shared across paths and it needs to be cleaned up for reuse.

I think the hw_handlers setting up the requests is not so fun. Maybe the 
block layer scsi_ioctl code could be reworked a little so that it could 
set some of that up for us since it is very similar. My hw handler was 
basically cutting and pasting of that code with some large 
simplifcations. I am working on a Target Port Group handler that is 
again cutting and pasting more code.

> 
> Good solutions solicited ;-)
> 
> 
> Sincerely,
>     Lars Marowsky-Brée <lmb@suse.de>
> 

Re: New -udm?

[Mike Christie]

Mike Christie wrote:
> Lars Marowsky-Bree wrote:
> 
>> On 2005-04-10T18:14:44, Dave Olien <dmo@osdl.org> wrote:
>>
>>
>>> You're correct.  I'll rewrite it on Thursday this week.
>>> I'll use the same methods Lars used in the dm-emc.c
>>
>>
>>
>> Note that dm-emc.c also would need to pre-allocate it's requests, but
>> doesn't right now :/
>>
>> Pre-allocating the requests sucks: Either we pre-allocate for _every_
>> path we might potentially need to send the request down on, or fix up
>> the request for the path we sent it down on (which would require us to
>> use internal knowledge about the req structs we're not supposed to
>> have).
> 
> 
> what is wrong with what you have now where you utilize the queue/path's 
> mempool by doing a blk_get_request with GFP_WAIT? 

or were you talking about the page for data backing which does not have 
a mempool like the bios (not per queue though) and requests.

Is that "fix up the
> request..." comment meaning that you do not like to access the request 
> structure for that code, or was it meaning that you have one request 
> that is shared across paths and it needs to be cleaned up for reuse.
> 
> I think the hw_handlers setting up the requests is not so fun. Maybe the 
> block layer scsi_ioctl code could be reworked a little so that it could 
> set some of that up for us since it is very similar. My hw handler was 
> basically cutting and pasting of that code with some large 
> simplifcations. I am working on a Target Port Group handler that is 
> again cutting and pasting more code.
> 
>>
>> Good solutions solicited ;-)
>>
>>
>> Sincerely,
>>     Lars Marowsky-Brée <lmb@suse.de>
>>
> 
> 
> -- 
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

Re: New -udm?

[Mike Christie]

Mike Christie wrote:
> Lars Marowsky-Bree wrote:
> 
>> On 2005-04-10T18:14:44, Dave Olien <dmo@osdl.org> wrote:
>>
>>
>>> You're correct.  I'll rewrite it on Thursday this week.
>>> I'll use the same methods Lars used in the dm-emc.c
>>
>>
>>
>> Note that dm-emc.c also would need to pre-allocate it's requests, but
>> doesn't right now :/
>>
>> Pre-allocating the requests sucks: Either we pre-allocate for _every_
>> path we might potentially need to send the request down on, or fix up
>> the request for the path we sent it down on (which would require us to
>> use internal knowledge about the req structs we're not supposed to
>> have).
> 
> 
> what is wrong with what you have now where you utilize the queue/path's 
> mempool by doing a blk_get_request with GFP_WAIT? Is that "fix up the 
> request..." comment meaning that you do not like to access the request 
> structure for that code, or was it meaning that you have one request 
> that is shared across paths and it needs to be cleaned up for reuse.
> 
> I think the hw_handlers setting up the requests is not so fun. Maybe the 
> block layer scsi_ioctl code could be reworked a little so that it could 
> set some of that up for us since it is very similar.

it is similar, but where the sgio paths allocated with GFP_KERNEL I used 
atomics. For the page allocation (which is way to large for the lsi 
command) I originally just preallocated a page but that seemed to 
wasteful. All I really needed to preallocate was my mode page struct and 
done a virt_to_page to get the page pointer to pass to bio_add_page (I 
wasn't sure if I could do that at the time), I guess? Since I ended up 
failing over at the controller level the preallocation would not even 
have to be per path so it would not have been that much waste.

Re: New -udm?

[Lars Marowsky-Bree]

On 2005-04-11T02:27:11, Mike Christie <mikenc@us.ibm.com> wrote:

> what is wrong with what you have now where you utilize the queue/path's 
> mempool by doing a blk_get_request with GFP_WAIT? 

... what if it's trying to free memory by going to swap on multipath,
and can't, because we're blocked on getting the request with
GFP_WAIT...?

Your patch helps, because it means we need less memory.

But, ultimately, we ought to preallocate the requests already when the
hw-handler is initialized for a map (because presumably at that time
we'll have enough memory, or can just fail the table setup). From that
point on, our memory useage should not grow.


Sincerely,
    Lars Marowsky-Brée <lmb@suse.de>

-- 
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business

Re: New -udm?

[Mike Christie]

Lars Marowsky-Bree wrote:
> On 2005-04-11T02:27:11, Mike Christie <mikenc@us.ibm.com> wrote:
> 
> 
>>what is wrong with what you have now where you utilize the queue/path's 
>>mempool by doing a blk_get_request with GFP_WAIT? 
> 
> 
> ... what if it's trying to free memory by going to swap on multipath,
> and can't, because we're blocked on getting the request with
> GFP_WAIT...?

GFP_WAIT does not casue IOs though. That is the difference between 
waiing on GFP_KERNEL and GFP_WAIT I thought. GFP_KERNEL can cause a page 
write out which you wait on and then there is a problem since it could 
be to the same disk you are trying to recover. But if you are just 
waiting for something to be returned to the mempool like with GFP_WAIT + 
blk_get_request you should be ok as long as the code below you 
eventually give up their resources and frees the requests you are 
waiting on?

> 
> Your patch helps, because it means we need less memory.
> 
> But, ultimately, we ought to preallocate the requests already when the
> hw-handler is initialized for a map (because presumably at that time
> we'll have enough memory, or can just fail the table setup). From that
> point on, our memory useage should not grow.
> 
> 
> Sincerely,
>     Lars Marowsky-Brée <lmb@suse.de>
>