2013-03-26 16:03:22

by Philip J. Kelleher

[permalink] [raw]
Subject: [PATCHv2 01/02] block: removes dynamic allocation on stack

From: Philip J Kelleher <[email protected]>

Removing the dynamic allocation on the stack.

Signed-off-by: Philip J Kelleher <[email protected]>
-------------------------------------------------------------------------------
o Version 2 consists of the error checking that was foolishly left out.
o Added error checking to multiple functions to support the dynamically
allocated list_head array.


diff -uprN -X linux-block/Documentation/dontdiff linux-block-vanilla/drivers/block/rsxx/core.c linux-block/drivers/block/rsxx/core.c
--- linux-block-vanilla/drivers/block/rsxx/core.c 2013-03-26 09:56:33.508217353 -0500
+++ linux-block/drivers/block/rsxx/core.c 2013-03-26 10:00:21.211964584 -0500
@@ -323,10 +323,11 @@ static int card_shutdown(struct rsxx_car
return 0;
}

-static void rsxx_eeh_frozen(struct pci_dev *dev)
+static int rsxx_eeh_frozen(struct pci_dev *dev)
{
struct rsxx_cardinfo *card = pci_get_drvdata(dev);
int i;
+ int st;

dev_warn(&dev->dev, "IBM FlashSystem PCI: preparing for slot reset.\n");

@@ -342,7 +343,9 @@ static void rsxx_eeh_frozen(struct pci_d

pci_disable_device(dev);

- rsxx_eeh_save_issued_dmas(card);
+ st = rsxx_eeh_save_issued_dmas(card);
+ if (st)
+ return st;

rsxx_eeh_save_issued_creg(card);

@@ -356,6 +359,8 @@ static void rsxx_eeh_frozen(struct pci_d
card->ctrl[i].cmd.buf,
card->ctrl[i].cmd.dma_addr);
}
+
+ return 0;
}

static void rsxx_eeh_failure(struct pci_dev *dev)
@@ -399,6 +404,8 @@ static int rsxx_eeh_fifo_flush_poll(stru
static pci_ers_result_t rsxx_error_detected(struct pci_dev *dev,
enum pci_channel_state error)
{
+ int st;
+
if (dev->revision < RSXX_EEH_SUPPORT)
return PCI_ERS_RESULT_NONE;

@@ -407,7 +414,13 @@ static pci_ers_result_t rsxx_error_detec
return PCI_ERS_RESULT_DISCONNECT;
}

- rsxx_eeh_frozen(dev);
+ st = rsxx_eeh_frozen(dev);
+ if (st) {
+ dev_err(&dev->dev, "Slot reset setup failed\n");
+ rsxx_eeh_failure(dev);
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
return PCI_ERS_RESULT_NEED_RESET;
}

diff -uprN -X linux-block/Documentation/dontdiff linux-block-vanilla/drivers/block/rsxx/dma.c linux-block/drivers/block/rsxx/dma.c
--- linux-block-vanilla/drivers/block/rsxx/dma.c 2013-03-26 09:57:10.071023932 -0500
+++ linux-block/drivers/block/rsxx/dma.c 2013-03-26 09:57:22.186976829 -0500
@@ -980,7 +980,7 @@ void rsxx_dma_destroy(struct rsxx_cardin
}
}

-void rsxx_eeh_save_issued_dmas(struct rsxx_cardinfo *card)
+int rsxx_eeh_save_issued_dmas(struct rsxx_cardinfo *card)
{
int i;
int j;
@@ -990,6 +990,8 @@ void rsxx_eeh_save_issued_dmas(struct rs

issued_dmas = kzalloc(sizeof(*issued_dmas) * card->n_targets,
GFP_KERNEL);
+ if (!issued_dmas)
+ return -ENOMEM;

for (i = 0; i < card->n_targets; i++) {
INIT_LIST_HEAD(&issued_dmas[i]);
@@ -1030,6 +1032,8 @@ void rsxx_eeh_save_issued_dmas(struct rs
}

kfree(issued_dmas);
+
+ return 0;
}

void rsxx_eeh_cancel_dmas(struct rsxx_cardinfo *card)
diff -uprN -X linux-block/Documentation/dontdiff linux-block-vanilla/drivers/block/rsxx/rsxx_priv.h linux-block/drivers/block/rsxx/rsxx_priv.h
--- linux-block-vanilla/drivers/block/rsxx/rsxx_priv.h 2013-03-26 09:56:33.531059843 -0500
+++ linux-block/drivers/block/rsxx/rsxx_priv.h 2013-03-26 09:57:22.192990813 -0500
@@ -381,7 +381,7 @@ int rsxx_dma_queue_bio(struct rsxx_cardi
rsxx_dma_cb cb,
void *cb_data);
int rsxx_hw_buffers_init(struct pci_dev *dev, struct rsxx_dma_ctrl *ctrl);
-void rsxx_eeh_save_issued_dmas(struct rsxx_cardinfo *card);
+int rsxx_eeh_save_issued_dmas(struct rsxx_cardinfo *card);
void rsxx_eeh_cancel_dmas(struct rsxx_cardinfo *card);
int rsxx_eeh_remap_dmas(struct rsxx_cardinfo *card);


2013-03-26 20:51:44

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCHv2 01/02] block: removes dynamic allocation on stack

On Tue, Mar 26 2013, Philip J. Kelleher wrote:
> From: Philip J Kelleher <[email protected]>
>
> Removing the dynamic allocation on the stack.
>
> Signed-off-by: Philip J Kelleher <[email protected]>
> -------------------------------------------------------------------------------
> o Version 2 consists of the error checking that was foolishly left out.
> o Added error checking to multiple functions to support the dynamically
> allocated list_head array.

I've been fixing up your patch formats for a while now, lets please just
improve on it a little bit. First of all, your subject line should be
prefixed with the driver name. So just use rsxx or similar. Secondly,
for this patch, you are using the same subject line as the original
patch. This is a fixup patch, it should be labelled as such. A proper
subject line for this patch would be:

rsxx: enable error return of rsxx_eeh_save_issued_dmas()

and the body could be:

Commit d8d595df introduced a bug where we did not check for a NULL
return from kmalloc(). Make rsxx_eeh_save_issued_dmas() return an
error for that case, and make the callers handle that.

You appear to put the description of the patch in a section below your
signed-off-by, which doesn't really work. Your --- divider gets broken
with git am as well.


--
Jens Axboe