* [PATCH] scsi_dh_rdac: Changing the memory allocation flag from GFP_KERNEL to GFP_NOIO
@ 2010-01-05 17:20 Moger, Babu
2010-01-27 22:19 ` [dm-devel] " Chandra Seetharaman
2010-11-04 15:14 ` Mike Snitzer
0 siblings, 2 replies; 4+ messages in thread
From: Moger, Babu @ 2010-01-05 17:20 UTC (permalink / raw)
To: linux-scsi@vger.kernel.org, dm-devel@redhat.com
Cc: michaelc@cs.wisc.edu, Rob Evers, Chandra Seetharaman
This patch changes the memory allocation flag from GFP_KERNEL to GFP_NOIO which is more appropriate.
This was noticed by Mike Christie and Rob Evers..
Mike Christie wrote:
> Rob Evers wrote:
>> +
>> +static int queue_mode_select(struct scsi_device *sdev,
>> + activate_complete fn, void *data)
>> +{
>> + struct rdac_queue_data *qdata;
>> + struct rdac_controller *ctlr;
>> +
>> + qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
>> + if (!qdata)
>> + return SCSI_DH_RETRY;
>> +
>
> It looks like this is called in the main IO path. A failed IO could cause, the path to go down,
> then we would call this to active a new path. If the system needed to write out pages to the same
> disk that just had the failed path so it can allocate memory then GFP_KERNEL would be bad to use here.
> You would use a mempool and still use something like GFP_NOIO. If a mempool is not possible for some
> reason, then you would want to still use GFP_NOIO.
Using mempool would become bit more complex here because I cannot assume about the number of elements
during the allocation. I would use GFP_NOIO.
CC: Mike Christie <michaelc@cs.wisc.edu>
CC: Rob Evers <revers@redhat.com>
CC: Chandra Seetharaman <chandra.seetharaman@us.ibm.com>
Signed-off-by: Babu Moger <babu.moger@lsi.com>
---
--- linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c.orig 2010-01-05 11:49:22.000000000 -0400
+++ linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c 2010-01-05 11:53:13.000000000 -0400
@@ -624,7 +624,7 @@ static int queue_mode_select(struct scsi
struct rdac_queue_data *qdata;
struct rdac_controller *ctlr;
- qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
+ qdata = kzalloc(sizeof(*qdata), GFP_NOIO);
if (!qdata)
return SCSI_DH_RETRY;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [dm-devel] [PATCH] scsi_dh_rdac: Changing the memory allocation flag from GFP_KERNEL to GFP_NOIO
2010-01-05 17:20 [PATCH] scsi_dh_rdac: Changing the memory allocation flag from GFP_KERNEL to GFP_NOIO Moger, Babu
@ 2010-01-27 22:19 ` Chandra Seetharaman
2010-11-04 15:14 ` Mike Snitzer
1 sibling, 0 replies; 4+ messages in thread
From: Chandra Seetharaman @ 2010-01-27 22:19 UTC (permalink / raw)
To: device-mapper development
Cc: linux-scsi@vger.kernel.org, Chandra Seetharaman,
michaelc@cs.wisc.edu, Rob Evers
Acked-by: Chandra Seetharaman <sekharan@us.ibm.com>
On Tue, 2010-01-05 at 10:20 -0700, Moger, Babu wrote:
> This patch changes the memory allocation flag from GFP_KERNEL to GFP_NOIO which is more appropriate.
>
> This was noticed by Mike Christie and Rob Evers..
> Mike Christie wrote:
> > Rob Evers wrote:
> >> +
> >> +static int queue_mode_select(struct scsi_device *sdev,
> >> + activate_complete fn, void *data)
> >> +{
> >> + struct rdac_queue_data *qdata;
> >> + struct rdac_controller *ctlr;
> >> +
> >> + qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
> >> + if (!qdata)
> >> + return SCSI_DH_RETRY;
> >> +
> >
> > It looks like this is called in the main IO path. A failed IO could cause, the path to go down,
> > then we would call this to active a new path. If the system needed to write out pages to the same
> > disk that just had the failed path so it can allocate memory then GFP_KERNEL would be bad to use here.
> > You would use a mempool and still use something like GFP_NOIO. If a mempool is not possible for some
> > reason, then you would want to still use GFP_NOIO.
>
> Using mempool would become bit more complex here because I cannot assume about the number of elements
> during the allocation. I would use GFP_NOIO.
>
> CC: Mike Christie <michaelc@cs.wisc.edu>
> CC: Rob Evers <revers@redhat.com>
> CC: Chandra Seetharaman <chandra.seetharaman@us.ibm.com>
> Signed-off-by: Babu Moger <babu.moger@lsi.com>
>
> ---
> --- linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c.orig 2010-01-05 11:49:22.000000000 -0400
> +++ linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c 2010-01-05 11:53:13.000000000 -0400
> @@ -624,7 +624,7 @@ static int queue_mode_select(struct scsi
> struct rdac_queue_data *qdata;
> struct rdac_controller *ctlr;
>
> - qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
> + qdata = kzalloc(sizeof(*qdata), GFP_NOIO);
> if (!qdata)
> return SCSI_DH_RETRY;
>
>
>
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: scsi_dh_rdac: Changing the memory allocation flag from GFP_KERNEL to GFP_NOIO
2010-01-05 17:20 [PATCH] scsi_dh_rdac: Changing the memory allocation flag from GFP_KERNEL to GFP_NOIO Moger, Babu
2010-01-27 22:19 ` [dm-devel] " Chandra Seetharaman
@ 2010-11-04 15:14 ` Mike Snitzer
2010-11-04 15:20 ` Moger, Babu
1 sibling, 1 reply; 4+ messages in thread
From: Mike Snitzer @ 2010-11-04 15:14 UTC (permalink / raw)
To: device-mapper development
Cc: linux-scsi@vger.kernel.org, Chandra Seetharaman,
michaelc@cs.wisc.edu, Rob Evers, Babu.Moger
This patch appears to have gotten lost in the shuffle but still seems
relevant. I found it while auditing dm-devel's patchwork, patch is
here: https://patchwork.kernel.org/patch/71073/
On Tue, Jan 05 2010 at 12:20pm -0500,
Moger, Babu <Babu.Moger@lsi.com> wrote:
> This patch changes the memory allocation flag from GFP_KERNEL to GFP_NOIO which is more appropriate.
>
> This was noticed by Mike Christie and Rob Evers..
> Mike Christie wrote:
> > Rob Evers wrote:
> >> +
> >> +static int queue_mode_select(struct scsi_device *sdev,
> >> + activate_complete fn, void *data)
> >> +{
> >> + struct rdac_queue_data *qdata;
> >> + struct rdac_controller *ctlr;
> >> +
> >> + qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
> >> + if (!qdata)
> >> + return SCSI_DH_RETRY;
> >> +
> >
> > It looks like this is called in the main IO path. A failed IO could cause, the path to go down,
> > then we would call this to active a new path. If the system needed to write out pages to the same
> > disk that just had the failed path so it can allocate memory then GFP_KERNEL would be bad to use here.
> > You would use a mempool and still use something like GFP_NOIO. If a mempool is not possible for some
> > reason, then you would want to still use GFP_NOIO.
>
> Using mempool would become bit more complex here because I cannot assume about the number of elements
> during the allocation. I would use GFP_NOIO.
>
> CC: Mike Christie <michaelc@cs.wisc.edu>
> CC: Rob Evers <revers@redhat.com>
> CC: Chandra Seetharaman <chandra.seetharaman@us.ibm.com>
> Signed-off-by: Babu Moger <babu.moger@lsi.com>
>
> ---
> --- linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c.orig 2010-01-05 11:49:22.000000000 -0400
> +++ linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c 2010-01-05 11:53:13.000000000 -0400
> @@ -624,7 +624,7 @@ static int queue_mode_select(struct scsi
> struct rdac_queue_data *qdata;
> struct rdac_controller *ctlr;
>
> - qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
> + qdata = kzalloc(sizeof(*qdata), GFP_NOIO);
> if (!qdata)
> return SCSI_DH_RETRY;
>
>
>
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: scsi_dh_rdac: Changing the memory allocation flag from GFP_KERNEL to GFP_NOIO
2010-11-04 15:14 ` Mike Snitzer
@ 2010-11-04 15:20 ` Moger, Babu
0 siblings, 0 replies; 4+ messages in thread
From: Moger, Babu @ 2010-11-04 15:20 UTC (permalink / raw)
To: Mike Snitzer, device-mapper development
Cc: Seetharaman, michaelc@cs.wisc.edu, Rob Evers, Chandra,
linux-scsi@vger.kernel.org
> -----Original Message-----
> From: Mike Snitzer [mailto:snitzer@redhat.com]
> Sent: Thursday, November 04, 2010 10:15 AM
> To: device-mapper development
> Cc: linux-scsi@vger.kernel.org; Chandra Seetharaman;
> michaelc@cs.wisc.edu; Rob Evers; Moger, Babu
> Subject: Re: scsi_dh_rdac: Changing the memory allocation flag from
> GFP_KERNEL to GFP_NOIO
>
> This patch appears to have gotten lost in the shuffle but still seems
> relevant. I found it while auditing dm-devel's patchwork, patch is
> here: https://patchwork.kernel.org/patch/71073/
Yes, Please.. We need this patch..
>
> On Tue, Jan 05 2010 at 12:20pm -0500,
> Moger, Babu <Babu.Moger@lsi.com> wrote:
>
> > This patch changes the memory allocation flag from GFP_KERNEL to
> GFP_NOIO which is more appropriate.
> >
> > This was noticed by Mike Christie and Rob Evers..
> > Mike Christie wrote:
> > > Rob Evers wrote:
> > >> +
> > >> +static int queue_mode_select(struct scsi_device *sdev,
> > >> + activate_complete fn, void *data)
> > >> +{
> > >> + struct rdac_queue_data *qdata;
> > >> + struct rdac_controller *ctlr;
> > >> +
> > >> + qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
> > >> + if (!qdata)
> > >> + return SCSI_DH_RETRY;
> > >> +
> > >
> > > It looks like this is called in the main IO path. A failed IO could
> cause, the path to go down,
> > > then we would call this to active a new path. If the system needed
> to write out pages to the same
> > > disk that just had the failed path so it can allocate memory then
> GFP_KERNEL would be bad to use here.
> > > You would use a mempool and still use something like GFP_NOIO. If a
> mempool is not possible for some
> > > reason, then you would want to still use GFP_NOIO.
> >
> > Using mempool would become bit more complex here because I cannot
> assume about the number of elements
> > during the allocation. I would use GFP_NOIO.
> >
> > CC: Mike Christie <michaelc@cs.wisc.edu>
> > CC: Rob Evers <revers@redhat.com>
> > CC: Chandra Seetharaman <chandra.seetharaman@us.ibm.com>
> > Signed-off-by: Babu Moger <babu.moger@lsi.com>
> >
> > ---
> > --- linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c.orig
> 2010-01-05 11:49:22.000000000 -0400
> > +++ linux-2.6.33-rc2/drivers/scsi/device_handler/scsi_dh_rdac.c 2010-
> 01-05 11:53:13.000000000 -0400
> > @@ -624,7 +624,7 @@ static int queue_mode_select(struct scsi
> > struct rdac_queue_data *qdata;
> > struct rdac_controller *ctlr;
> >
> > - qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
> > + qdata = kzalloc(sizeof(*qdata), GFP_NOIO);
> > if (!qdata)
> > return SCSI_DH_RETRY;
> >
> >
> >
> >
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-04 15:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-05 17:20 [PATCH] scsi_dh_rdac: Changing the memory allocation flag from GFP_KERNEL to GFP_NOIO Moger, Babu
2010-01-27 22:19 ` [dm-devel] " Chandra Seetharaman
2010-11-04 15:14 ` Mike Snitzer
2010-11-04 15:20 ` Moger, Babu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).