Re: [PATCH v2 5/8] mpt3sas: Fix _transport_smp_handler() error path
On Fri, Jun 15, 2018 at 02:41:58PM -0700, Bart Van Assche wrote: > This patch avoids that smatch complains about a double unlock on > ioc->transport_cmds.mutex. Looks good, Reviewed-by: Christoph Hellwig
Re: [PATCH-next] scsi: libsas: dynamically allocate and free ata host
Jason, > So we have to change this embedded static ata host to a dynamically > allocated ata host and initialize the ->kref member. To use > ata_host_get() and ata_host_put() in libsas, we need to move the > declaration of these functions to the public libata.h and export them. Took a while for all the prerequisites to materialize. I just rebased 4.19/scsi-queue to v4.18-rc1 and applied your patch. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [patch] sg: clean up gfp_mask in sg_build_indirect
Jeff, > And there's only one user of the gfp_mask. Just or in the __GFP_ZERO > flag at the top of the function and be done with it. Applied to 4.19/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 2/2] qla2xxx: remove irq save in qla2x00_poll()
Himanshu,
>> In commit d2ba5675d899 ("[SCSI] qla2xxx: Disable local-interrupts while
>> polling for RISC status.") added a local_irq_disable() before invoking
>> the ->intr_handler callback. The function, which was used in this
>> callback, did not disable interrupts while acquiring the spin_lock so a
>> deadlock was possible and this change was one possible solution.
Applied to 4.19/scsi-queue. Thanks!
--
Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 2/2] qla2xxx: remove irq save in qla2x00_poll()
> On May 4, 2018, at 7:50 AM, Sebastian Andrzej Siewior
> wrote:
>
> In commit d2ba5675d899 ("[SCSI] qla2xxx: Disable local-interrupts while
> polling for RISC status.") added a local_irq_disable() before invoking
> the ->intr_handler callback. The function, which was used in this
> callback, did not disable interrupts while acquiring the spin_lock so a
> deadlock was possible and this change was one possible solution.
>
> The function in question was qla2300_intr_handler() and is using
> spin_lock_irqsave() since commit 43fac4d97a1a ("[SCSI] qla2xxx: Resolve
> a performance issue in interrupt").
> I checked all other ->intr_handler callbacks and all of them use the
> irqsave variant so it is safe to remove the local_irq_save() block now.
>
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> drivers/scsi/qla2xxx/qla_inline.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_inline.h
> b/drivers/scsi/qla2xxx/qla_inline.h
> index 37ae0f6d8ae5..bcbdf28bd7b9 100644
> --- a/drivers/scsi/qla2xxx/qla_inline.h
> +++ b/drivers/scsi/qla2xxx/qla_inline.h
> @@ -58,14 +58,12 @@ qla2x00_debounce_register(volatile uint16_t __iomem *addr)
> static inline void
> qla2x00_poll(struct rsp_que *rsp)
> {
> - unsigned long flags;
> struct qla_hw_data *ha = rsp->hw;
> - local_irq_save(flags);
> +
> if (IS_P3P_TYPE(ha))
> qla82xx_poll(0, rsp);
> else
> ha->isp_ops->intr_handler(0, rsp);
> - local_irq_restore(flags);
> }
>
> static inline uint8_t *
> --
> 2.17.0
>
Looks good
Acked-by: Himanshu Madhani
Thanks,
- Himanshu
[PATCH 3/5] smartpqi: add inspur advantech ids
From: Kevin Barnett
- add support for these new device IDs:
Advantech MIC-8312BridgeB
INSPUR PM8204-2GB
INSPUR PM8204-4GB
INSPUR PM8222-SHBA
Reviewed-by: Scott Benesh
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi_init.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index 1593ee343a2b..8b70b879735e 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6837,6 +6837,18 @@ static const struct pci_device_id pqi_pci_id_table[] = {
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
0x1bd4, 0x0048)
},
+ {
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+ 0x1bd4, 0x004a)
+ },
+ {
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+ 0x1bd4, 0x004b)
+ },
+ {
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+ 0x1bd4, 0x004c)
+ },
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_VENDOR_ID_ADAPTEC2, 0x0110)
@@ -6961,6 +6973,10 @@ static const struct pci_device_id pqi_pci_id_table[] = {
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_VENDOR_ID_ADAPTEC2, 0x1380)
},
+ {
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+ PCI_VENDOR_ID_ADVANTECH, 0x8312)
+ },
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
PCI_VENDOR_ID_DELL, 0x1fe0)
[PATCH 4/5] smartpqi: fix critical ARM issue reading PQI index registers
From: Kevin Barnett
- use the readl() kernel function to read all index
registers. For ARM systems, this function includes a
read memory barrier that eliminates ci/pi corruption.
Reviewed-by: Scott Benesh
Reviewed-by: Scott Teel
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi.h | 10 ---
drivers/scsi/smartpqi/smartpqi_init.c | 45 ++---
2 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index a8e7c4d48061..e97bf2670315 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -583,8 +583,8 @@ struct pqi_admin_queues_aligned {
struct pqi_admin_queues {
void*iq_element_array;
void*oq_element_array;
- volatile pqi_index_t *iq_ci;
- volatile pqi_index_t *oq_pi;
+ pqi_index_t *iq_ci;
+ pqi_index_t __iomem *oq_pi;
dma_addr_t iq_element_array_bus_addr;
dma_addr_t oq_element_array_bus_addr;
dma_addr_t iq_ci_bus_addr;
@@ -608,8 +608,8 @@ struct pqi_queue_group {
dma_addr_t oq_element_array_bus_addr;
__le32 __iomem *iq_pi[2];
pqi_index_t iq_pi_copy[2];
- volatile pqi_index_t *iq_ci[2];
- volatile pqi_index_t *oq_pi;
+ pqi_index_t __iomem *iq_ci[2];
+ pqi_index_t __iomem *oq_pi;
dma_addr_t iq_ci_bus_addr[2];
dma_addr_t oq_pi_bus_addr;
__le32 __iomem *oq_ci;
@@ -622,7 +622,7 @@ struct pqi_event_queue {
u16 oq_id;
u16 int_msg_num;
void*oq_element_array;
- volatile pqi_index_t *oq_pi;
+ pqi_index_t __iomem *oq_pi;
dma_addr_t oq_element_array_bus_addr;
dma_addr_t oq_pi_bus_addr;
__le32 __iomem *oq_ci;
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index 8b70b879735e..b4a685ed9ed1 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -2703,7 +2703,7 @@ static unsigned int pqi_process_io_intr(struct
pqi_ctrl_info *ctrl_info,
oq_ci = queue_group->oq_ci_copy;
while (1) {
- oq_pi = *queue_group->oq_pi;
+ oq_pi = readl(queue_group->oq_pi);
if (oq_pi == oq_ci)
break;
@@ -2794,7 +2794,7 @@ static void pqi_send_event_ack(struct pqi_ctrl_info
*ctrl_info,
spin_lock_irqsave(_group->submit_lock[RAID_PATH], flags);
iq_pi = queue_group->iq_pi_copy[RAID_PATH];
- iq_ci = *queue_group->iq_ci[RAID_PATH];
+ iq_ci = readl(queue_group->iq_ci[RAID_PATH]);
if (pqi_num_elements_free(iq_pi, iq_ci,
ctrl_info->num_elements_per_iq))
@@ -2953,7 +2953,7 @@ static unsigned int pqi_process_event_intr(struct
pqi_ctrl_info *ctrl_info)
oq_ci = event_queue->oq_ci_copy;
while (1) {
- oq_pi = *event_queue->oq_pi;
+ oq_pi = readl(event_queue->oq_pi);
if (oq_pi == oq_ci)
break;
@@ -3177,7 +3177,7 @@ static int pqi_alloc_operational_queues(struct
pqi_ctrl_info *ctrl_info)
size_t element_array_length_per_iq;
size_t element_array_length_per_oq;
void *element_array;
- void *next_queue_index;
+ void __iomem *next_queue_index;
void *aligned_pointer;
unsigned int num_inbound_queues;
unsigned int num_outbound_queues;
@@ -3273,7 +3273,7 @@ static int pqi_alloc_operational_queues(struct
pqi_ctrl_info *ctrl_info)
element_array += PQI_NUM_EVENT_QUEUE_ELEMENTS *
PQI_EVENT_OQ_ELEMENT_LENGTH;
- next_queue_index = PTR_ALIGN(element_array,
+ next_queue_index = (void __iomem *)PTR_ALIGN(element_array,
PQI_OPERATIONAL_INDEX_ALIGNMENT);
for (i = 0; i < ctrl_info->num_queue_groups; i++) {
@@ -3281,21 +3281,24 @@ static int pqi_alloc_operational_queues(struct
pqi_ctrl_info *ctrl_info)
queue_group->iq_ci[RAID_PATH] = next_queue_index;
queue_group->iq_ci_bus_addr[RAID_PATH] =
ctrl_info->queue_memory_base_dma_handle +
- (next_queue_index - ctrl_info->queue_memory_base);
+ (next_queue_index -
+ (void __iomem *)ctrl_info->queue_memory_base);
next_queue_index += sizeof(pqi_index_t);
next_queue_index = PTR_ALIGN(next_queue_index,
PQI_OPERATIONAL_INDEX_ALIGNMENT);
queue_group->iq_ci[AIO_PATH] = next_queue_index;
queue_group->iq_ci_bus_addr[AIO_PATH] =
ctrl_info->queue_memory_base_dma_handle +
- (next_queue_index -
[PATCH 5/5] smartpqi: bump driver version to 1.1.4-130
Reviewed-by: Scott Benesh Signed-off-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index b4a685ed9ed1..2112ea6723c6 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -40,11 +40,11 @@ #define BUILD_TIMESTAMP #endif -#define DRIVER_VERSION "1.1.4-115" +#define DRIVER_VERSION "1.1.4-130" #define DRIVER_MAJOR 1 #define DRIVER_MINOR 1 #define DRIVER_RELEASE 4 -#define DRIVER_REVISION115 +#define DRIVER_REVISION130 #define DRIVER_NAME"Microsemi PQI Driver (v" \ DRIVER_VERSION BUILD_TIMESTAMP ")"
[PATCH 2/5] smartpqi: improve error checking for sync requests
From: Kevin Barnett
- detect rare error cases for synchronous requests down the RAID path
- retry INQUIRY of VPD page 0 sent to an HBA drive if the command
failed due to an abort.
Reviewed-by: Scott Benesh
Reviewed-by: Scott Teel
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi.h |2 +
drivers/scsi/smartpqi/smartpqi_init.c | 54 +++--
2 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index dc3a0542a2e8..a8e7c4d48061 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -483,6 +483,8 @@ struct pqi_raid_error_info {
#define CISS_CMD_STATUS_TMF0xd
#define CISS_CMD_STATUS_AIO_DISABLED 0xe
+#define PQI_CMD_STATUS_ABORTED CISS_CMD_STATUS_ABORTED
+
#define PQI_NUM_EVENT_QUEUE_ELEMENTS 32
#define PQI_EVENT_OQ_ELEMENT_LENGTHsizeof(struct pqi_event_response)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index 4036f65cbb72..1593ee343a2b 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -1197,20 +1197,30 @@ static void pqi_get_volume_status(struct pqi_ctrl_info
*ctrl_info,
device->volume_offline = volume_offline;
}
+#define PQI_INQUIRY_PAGE0_RETRIES 3
+
static int pqi_get_device_info(struct pqi_ctrl_info *ctrl_info,
struct pqi_scsi_dev *device)
{
int rc;
u8 *buffer;
+ unsigned int retries;
buffer = kmalloc(64, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
/* Send an inquiry to the device to see what it is. */
- rc = pqi_scsi_inquiry(ctrl_info, device->scsi3addr, 0, buffer, 64);
- if (rc)
- goto out;
+ for (retries = 0;;) {
+ rc = pqi_scsi_inquiry(ctrl_info, device->scsi3addr, 0,
+ buffer, 64);
+ if (rc == 0)
+ break;
+ if (pqi_is_logical_device(device) ||
+ rc != PQI_CMD_STATUS_ABORTED ||
+ ++retries > PQI_INQUIRY_PAGE0_RETRIES)
+ goto out;
+ }
scsi_sanitize_inquiry_string([8], 8);
scsi_sanitize_inquiry_string([16], 16);
@@ -3621,6 +3631,29 @@ static void pqi_raid_synchronous_complete(struct
pqi_io_request *io_request,
complete(waiting);
}
+static int pqi_process_raid_io_error_synchronous(struct pqi_raid_error_info
+ *error_info)
+{
+ int rc = -EIO;
+
+ switch (error_info->data_out_result) {
+ case PQI_DATA_IN_OUT_GOOD:
+ if (error_info->status == SAM_STAT_GOOD)
+ rc = 0;
+ break;
+ case PQI_DATA_IN_OUT_UNDERFLOW:
+ if (error_info->status == SAM_STAT_GOOD ||
+ error_info->status == SAM_STAT_CHECK_CONDITION)
+ rc = 0;
+ break;
+ case PQI_DATA_IN_OUT_ABORTED:
+ rc = PQI_CMD_STATUS_ABORTED;
+ break;
+ }
+
+ return rc;
+}
+
static int pqi_submit_raid_request_synchronous(struct pqi_ctrl_info *ctrl_info,
struct pqi_iu_header *request, unsigned int flags,
struct pqi_raid_error_info *error_info, unsigned long timeout_msecs)
@@ -3710,19 +3743,8 @@ static int pqi_submit_raid_request_synchronous(struct
pqi_ctrl_info *ctrl_info,
else
memset(error_info, 0, sizeof(*error_info));
} else if (rc == 0 && io_request->error_info) {
- u8 scsi_status;
- struct pqi_raid_error_info *raid_error_info;
-
- raid_error_info = io_request->error_info;
- scsi_status = raid_error_info->status;
-
- if (scsi_status == SAM_STAT_CHECK_CONDITION &&
- raid_error_info->data_out_result ==
- PQI_DATA_IN_OUT_UNDERFLOW)
- scsi_status = SAM_STAT_GOOD;
-
- if (scsi_status != SAM_STAT_GOOD)
- rc = -EIO;
+ rc = pqi_process_raid_io_error_synchronous(
+ io_request->error_info);
}
pqi_free_io_request(io_request);
[PATCH 0/5] smartpqi updates
These patches are based on Linus's tree The changes are: - improve handling for sync requests ensure controller is ready for requests - improve-error-checking-for-sync-requests detect rare error cases for synchronous requests - add more supported devices - fix critical ARM issue reading PQI index registers fix ci/pi index corruption on ARM platforms - bump driver version --- Don Brace (1): smartpqi: bump driver version to 1.1.4-130 Kevin Barnett (4): smartpqi: improve handling for sync requests smartpqi: improve error checking for sync requests smartpqi: add inspur advantech ids smartpqi: fix critical ARM issue reading PQI index registers drivers/scsi/smartpqi/smartpqi.h | 12 +- drivers/scsi/smartpqi/smartpqi_init.c | 160 - 2 files changed, 104 insertions(+), 68 deletions(-) -- Signature
[PATCH 1/5] smartpqi: improve handling for sync requests
From: Kevin Barnett
- decrement the active thread count after
the synchronous request was submitted to the controller but before
the driver blocks to wait for the request to complete.
Reviewed-by: Scott Benesh
Reviewed-by: Scott Teel
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi_init.c | 55 ++---
1 file changed, 23 insertions(+), 32 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index b78d20b74ed8..4036f65cbb72 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -3621,43 +3621,16 @@ static void pqi_raid_synchronous_complete(struct
pqi_io_request *io_request,
complete(waiting);
}
-static int pqi_submit_raid_request_synchronous_with_io_request(
- struct pqi_ctrl_info *ctrl_info, struct pqi_io_request *io_request,
- unsigned long timeout_msecs)
-{
- int rc = 0;
- DECLARE_COMPLETION_ONSTACK(wait);
-
- io_request->io_complete_callback = pqi_raid_synchronous_complete;
- io_request->context =
-
- pqi_start_io(ctrl_info,
- _info->queue_groups[PQI_DEFAULT_QUEUE_GROUP], RAID_PATH,
- io_request);
-
- if (timeout_msecs == NO_TIMEOUT) {
- pqi_wait_for_completion_io(ctrl_info, );
- } else {
- if (!wait_for_completion_io_timeout(,
- msecs_to_jiffies(timeout_msecs))) {
- dev_warn(_info->pci_dev->dev,
- "command timed out\n");
- rc = -ETIMEDOUT;
- }
- }
-
- return rc;
-}
-
static int pqi_submit_raid_request_synchronous(struct pqi_ctrl_info *ctrl_info,
struct pqi_iu_header *request, unsigned int flags,
struct pqi_raid_error_info *error_info, unsigned long timeout_msecs)
{
- int rc;
+ int rc = 0;
struct pqi_io_request *io_request;
unsigned long start_jiffies;
unsigned long msecs_blocked;
size_t iu_length;
+ DECLARE_COMPLETION_ONSTACK(wait);
/*
* Note that specifying PQI_SYNC_FLAGS_INTERRUPTABLE and a timeout value
@@ -3686,11 +3659,13 @@ static int pqi_submit_raid_request_synchronous(struct
pqi_ctrl_info *ctrl_info,
pqi_ctrl_busy(ctrl_info);
timeout_msecs = pqi_wait_if_ctrl_blocked(ctrl_info, timeout_msecs);
if (timeout_msecs == 0) {
+ pqi_ctrl_unbusy(ctrl_info);
rc = -ETIMEDOUT;
goto out;
}
if (pqi_ctrl_offline(ctrl_info)) {
+ pqi_ctrl_unbusy(ctrl_info);
rc = -ENXIO;
goto out;
}
@@ -3708,8 +3683,25 @@ static int pqi_submit_raid_request_synchronous(struct
pqi_ctrl_info *ctrl_info,
PQI_REQUEST_HEADER_LENGTH;
memcpy(io_request->iu, request, iu_length);
- rc = pqi_submit_raid_request_synchronous_with_io_request(ctrl_info,
- io_request, timeout_msecs);
+ io_request->io_complete_callback = pqi_raid_synchronous_complete;
+ io_request->context =
+
+ pqi_start_io(ctrl_info,
+ _info->queue_groups[PQI_DEFAULT_QUEUE_GROUP], RAID_PATH,
+ io_request);
+
+ pqi_ctrl_unbusy(ctrl_info);
+
+ if (timeout_msecs == NO_TIMEOUT) {
+ pqi_wait_for_completion_io(ctrl_info, );
+ } else {
+ if (!wait_for_completion_io_timeout(,
+ msecs_to_jiffies(timeout_msecs))) {
+ dev_warn(_info->pci_dev->dev,
+ "command timed out\n");
+ rc = -ETIMEDOUT;
+ }
+ }
if (error_info) {
if (io_request->error_info)
@@ -3736,7 +3728,6 @@ static int pqi_submit_raid_request_synchronous(struct
pqi_ctrl_info *ctrl_info,
pqi_free_io_request(io_request);
out:
- pqi_ctrl_unbusy(ctrl_info);
up(_info->sync_request_sem);
return rc;
Re: [patch] sg: clean up gfp_mask in sg_build_indirect
On 2018-06-18 09:57 AM, Jeff Moyer wrote:
commit a45b599ad808c ("scsi: sg: allocate with __GFP_ZERO in
sg_build_indirect()") changed the call to alloc_pages to always use
__GFP_ZERO. Just above that, though, there was this:
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
gfp_mask |= __GFP_ZERO;
And there's only one user of the gfp_mask. Just or in the __GFP_ZERO
flag at the top of the function and be done with it.
Signed-off-by: Jeff Moyer
Acked-by: Douglas Gilbert
p.s. I'm not even sure why the original patch was committed -- it does
nothing for security. I assume that the zeroing isn't done for
CAP_SYS_ADMIN / CAP_SYS_RAWIO because of performance.
Yes.
(The history of
that zeroing being conditional goes way back to pre-git.) Was there any
performance measurement done after the commit a45b599ad808c was applied?
No. It was done in the name of security. Alexander Potapenko felt it was
more dangerous to see kernel memory via pseudo-random sequences pushed
to the sg driver's write()/read() async interface than it was to do
'cat /dev/mem'. He suggested some old distros gave non-root access to
/dev/sg* device nodes in some situations.
Then I added that other users of the SG_IO ioctl may have a similar
problem (e.g. ioctl(SG_IO) on a primary block device name and via the
bsg driver). That has lead to more list "noise".
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 53ae52dbff84..c926e07aabda 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1850,7 +1850,7 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp,
int buff_size)
int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems;
int sg_tablesize = sfp->parentdp->sg_tablesize;
int blk_size = buff_size, order;
- gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
+ gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO;
struct sg_device *sdp = sfp->parentdp;
if (blk_size < 0)
@@ -1880,9 +1880,6 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp,
int buff_size)
if (sdp->device->host->unchecked_isa_dma)
gfp_mask |= GFP_DMA;
- if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
- gfp_mask |= __GFP_ZERO;
-
order = get_order(num);
retry:
ret_sz = 1 << (PAGE_SHIFT + order);
@@ -1893,7 +1890,7 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp,
int buff_size)
num = (rem_sz > scatter_elem_sz_prev) ?
scatter_elem_sz_prev : rem_sz;
- schp->pages[k] = alloc_pages(gfp_mask | __GFP_ZERO, order);
+ schp->pages[k] = alloc_pages(gfp_mask, order);
if (!schp->pages[k])
goto out;
[patch] sg: clean up gfp_mask in sg_build_indirect
commit a45b599ad808c ("scsi: sg: allocate with __GFP_ZERO in
sg_build_indirect()") changed the call to alloc_pages to always use
__GFP_ZERO. Just above that, though, there was this:
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
gfp_mask |= __GFP_ZERO;
And there's only one user of the gfp_mask. Just or in the __GFP_ZERO
flag at the top of the function and be done with it.
Signed-off-by: Jeff Moyer
p.s. I'm not even sure why the original patch was committed -- it does
nothing for security. I assume that the zeroing isn't done for
CAP_SYS_ADMIN / CAP_SYS_RAWIO because of performance. (The history of
that zeroing being conditional goes way back to pre-git.) Was there any
performance measurement done after the commit a45b599ad808c was applied?
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 53ae52dbff84..c926e07aabda 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1850,7 +1850,7 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp,
int buff_size)
int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems;
int sg_tablesize = sfp->parentdp->sg_tablesize;
int blk_size = buff_size, order;
- gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
+ gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO;
struct sg_device *sdp = sfp->parentdp;
if (blk_size < 0)
@@ -1880,9 +1880,6 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp,
int buff_size)
if (sdp->device->host->unchecked_isa_dma)
gfp_mask |= GFP_DMA;
- if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
- gfp_mask |= __GFP_ZERO;
-
order = get_order(num);
retry:
ret_sz = 1 << (PAGE_SHIFT + order);
@@ -1893,7 +1890,7 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp,
int buff_size)
num = (rem_sz > scatter_elem_sz_prev) ?
scatter_elem_sz_prev : rem_sz;
- schp->pages[k] = alloc_pages(gfp_mask | __GFP_ZERO, order);
+ schp->pages[k] = alloc_pages(gfp_mask, order);
if (!schp->pages[k])
goto out;
Re: Obtain file's inode in ufshcd driver
Hi Roman, On Fri, Jun 1, 2018 at 4:25 AM Roman Storozhenko wrote: > > Hello everybody. > > I am modifying ufshcd driver: > https://elixir.bootlin.com/linux/v3.8/source/drivers/scsi/ufs/ufshcd.c > > I do a read of a file on ext4 filesystem and want to obtain the file's > inode. But it seems that there are no structures contatin any > references to inode at this level of abstraction. I'm no expert, I'm just answering so you actually have an answer, not because I have any good knowledge in this area. This is by design - by the time the data gets to the hardware the associated file is both irrelevant and unobtainable: the read or write might be metadata or housekeeping, the filesystem might be log-structured so it's reading or writing bits of 5 different files, the upper layer might be a swap partition or something more exotic, it might be part of a disk check, there might be software RAID between the filesystem and hardware, etc. Why do you need this data and what are you trying to accomplish here? > I looked into scsi_cmd structure and all the structures it references > to. Also I used ftrace to find on which level of abstraction inode > disappear but without any luck. Could you please say to me where I > could find the lowest level where inode is still present and the best > way how to pass it throughout linux io stack to the ufshcd driver? > Frankly speaking I need not struct inode as a whole but just its > number, so using reserved or unused fields of existing structures is > appropriate. The inode number is part of the filesystem so it wouldn't appear below that layer. Thanks, -- Julian Calaby Email: julian.cal...@gmail.com Profile: http://www.google.com/profiles/julian.calaby/
Re: [PATCH 01/33] TCMU PR: first commit to implement TCMU PR
On Mon, 18 Jun 2018 04:12:17 -0700, Christoph Hellwig wrote: > On Sun, Jun 17, 2018 at 12:40:56PM +0800, Zhu Lingshan wrote: > > Hello Mike and Christoph， > > Thanks Mike's comment inspired me, if I understand this correctly, it is > > suggested to implement this whole solution in kernel, avoid > > splitting PRG handling in both kernel and userspace, make it not that > > complex. I can try to implement this by sending OSD requests > > from kernel side, then we don't need tcmu-runner supporting is, no need to > > use netlink to exchange information with userspace, pure kernel code, seems > > much more simple. Do you think this may be better? > > Yes. And SuSE actually ships such a backend (originally writen by > Mike), although it would still need some work to get into shape. FWIW, the kernel backend is also shipped by petasan. I'd be happy to restart efforts with Mike to get this in shape for mainline. The current target_core_rbd PR implementation is suboptimal in that it duplicates quite a bit of protocol logic from target_core_pr. Cheers, David
Re: [PATCH 01/33] TCMU PR: first commit to implement TCMU PR
On 2018/6/18 19:12, Christoph Hellwig wrote: On Sun, Jun 17, 2018 at 12:40:56PM +0800, Zhu Lingshan wrote: Hello Mike and Christoph， Thanks Mike's comment inspired me, if I understand this correctly, it is suggested to implement this whole solution in kernel, avoid splitting PRG handling in both kernel and userspace, make it not that complex. I can try to implement this by sending OSD requests from kernel side, then we don't need tcmu-runner supporting is, no need to use netlink to exchange information with userspace, pure kernel code, seems much more simple. Do you think this may be better? Yes. And SuSE actually ships such a backend (originally writen by Mike), although it would still need some work to get into shape. Thanks, I will work on that. Thanks, BR Zhu Lingshan
Re: [PATCH 01/33] TCMU PR: first commit to implement TCMU PR
On Sun, Jun 17, 2018 at 12:40:56PM +0800, Zhu Lingshan wrote: > Hello Mike and Christoph， > Thanks Mike's comment inspired me, if I understand this correctly, it is > suggested to implement this whole solution in kernel, avoid > splitting PRG handling in both kernel and userspace, make it not that > complex. I can try to implement this by sending OSD requests > from kernel side, then we don't need tcmu-runner supporting is, no need to > use netlink to exchange information with userspace, pure kernel code, seems > much more simple. Do you think this may be better? Yes. And SuSE actually ships such a backend (originally writen by Mike), although it would still need some work to get into shape.
Re: [PATCH 01/33] TCMU PR: first commit to implement TCMU PR
On Sat, Jun 16, 2018 at 02:25:47PM -0500, Mike Christie wrote: > > Just wanted to make sure I understood this comment. In Lingshan's > > patches I think he was going to end up calling out to > > userspace/tcmu-runner and there he was going to make ceph calls which > > basically translate PGR operations to ceph requests. Are you saying we > > should just add some kernel module that makes the ceph calls? This would > > then avoid the mess of having the split PGR processing design in this > > patchset? > > Oh yeah, I meant to also ask if I understood you correctly above, then > did you also just want us to add the target_core_rbd module that did > READ/WRITE/VAAI commands too? In general I'd prefer to have generic APIs for it so that we could also front an NVMe target with the same ceph backend. But we can at least get that work started this way, and I'll look into helping with a proper API.
Re: [PATCH 01/33] TCMU PR: first commit to implement TCMU PR
On Sat, Jun 16, 2018 at 02:20:12PM -0500, Mike Christie wrote: > Just wanted to make sure I understood this comment. In Lingshan's > patches I think he was going to end up calling out to > userspace/tcmu-runner and there he was going to make ceph calls which > basically translate PGR operations to ceph requests. Are you saying we > should just add some kernel module that makes the ceph calls? This would > then avoid the mess of having the split PGR processing design in this > patchset? Yes.
