Re: [PATCH v2] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Hannes Reinecke
On 06/24/2015 08:54 AM, Seymour, Shane M wrote:
> 
> Convert DRIVER_ATTR macros to DRIVER_ATTR_RO requested by
> Greg KH. Also switched to using scnprintf instead of snprintf
> per Documentation/filesystems/sysfs.txt.
> 
> Suggested-by: Greg Kroah-Hartman 
> Signed-off-by: Shane Seymour 
> ---
> This patch was implemented on top of the previous patch to
> convert to using driver attr groups.
> 
> Changes from v1:
> - switched to scnprintf from sprintf after feedback from Sergey
> Senozhatsky.

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH][SCSI] hptiop: Support HighPoint RR36xx HBAs and Support SAS tape and SAS media changer

2015-06-24 Thread Hannes Reinecke
On 06/24/2015 05:41 AM, linux wrote:
> Support HighPoint RR36xx HBAs which are based on Marvell Frey.
> Support SAS tape and SAS media changer.
> 
> Signed-off-by: HighPoint Linux Team 
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] advansys: Do not byteswap sense_len on big endian

2015-06-24 Thread Geert Uytterhoeven
On big endian (e.g. m68k):

drivers/scsi/advansys.c: In function ‘adv_build_req’:
drivers/scsi/advansys.c:7806: warning: large integer implicitly truncated 
to unsigned type

Indeed, adv_scsi_req_q.sense_len is uchar, not __le32, and must never be
byteswapped.

Fixes: 811ddc057aac7228 ("advansys: use DMA-API for mapping sense buffer")
Signed-off-by: Geert Uytterhoeven 
---
 drivers/scsi/advansys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 4305178e4e0147ba..1c1cd657c380c2e0 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -7803,7 +7803,7 @@ adv_build_req(struct asc_board *boardp, struct scsi_cmnd 
*scp,
return ASC_BUSY;
}
scsiqp->sense_addr = cpu_to_le32(sense_addr);
-   scsiqp->sense_len = cpu_to_le32(SCSI_SENSE_BUFFERSIZE);
+   scsiqp->sense_len = SCSI_SENSE_BUFFERSIZE;
 
/* Build ADV_SCSI_REQ_Q */
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] advansys: Make sure ret and share_irq are initialized

2015-06-24 Thread Geert Uytterhoeven
If CONFIG_PCI=n:

drivers/scsi/advansys.c: In function ‘advansys_board_found’:
drivers/scsi/advansys.c:11037: warning: ‘ret’ is used uninitialized in this 
function
drivers/scsi/advansys.c:10929: warning: ‘share_irq’ may be used 
uninitialized in this function

If CONFIG_PCI=n, ret and share_irq are indeed not initialized.
Pre-initialize them to zero, and drop the now superfluous setting of
share_irq to zero.

Note that this is sort-of a false positive, as apparently
ASC_NARROW_BOARD(boardp) can only be false for PCI boards, e.g. if
CONFIG_PCI=y.

Signed-off-by: Geert Uytterhoeven 
---
 drivers/scsi/advansys.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 1c1cd657c380c2e0..a056226d3f35e863 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -10926,7 +10926,7 @@ static int advansys_board_found(struct Scsi_Host 
*shost, unsigned int iop,
struct asc_board *boardp = shost_priv(shost);
ASC_DVC_VAR *asc_dvc_varp = NULL;
ADV_DVC_VAR *adv_dvc_varp = NULL;
-   int share_irq, warn_code, ret;
+   int share_irq = 0, warn_code, ret = 0;
 
pdev = (bus_type == ASC_IS_PCI) ? to_pci_dev(boardp->dev) : NULL;
 
@@ -10987,11 +10987,9 @@ static int advansys_board_found(struct Scsi_Host 
*shost, unsigned int iop,
 #ifdef CONFIG_ISA
case ASC_IS_ISA:
shost->unchecked_isa_dma = true;
-   share_irq = 0;
break;
case ASC_IS_VL:
shost->unchecked_isa_dma = false;
-   share_irq = 0;
break;
case ASC_IS_EISA:
shost->unchecked_isa_dma = false;
@@ -11008,7 +11006,6 @@ static int advansys_board_found(struct Scsi_Host 
*shost, unsigned int iop,
shost_printk(KERN_ERR, shost, "unknown adapter type: "
"%d\n", asc_dvc_varp->bus_type);
shost->unchecked_isa_dma = false;
-   share_irq = 0;
break;
}
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] hpsa: fix an sprintf() overflow in the reset handler

2015-06-24 Thread Dan Carpenter
On Fri, Jun 19, 2015 at 07:13:47AM +, Seymour, Shane M wrote:
> With a size of 48 while it won't overflow since you're using snprintf the 
> string with a maximum value in %d:
> 
> echo -n "cmd 2147483647 RESET FAILED, new lockup detected" |wc -c
> 48

I actually just chose 48 because it was divisible by sizeof(long) on
64 bit.  The compiler is going to pad it out a bit anyway because of
alignment.

> 
> is 48 characters long without a null terminator on the string (and in the 
> unlikely event that it's somehow a the largest possible negative value it 
> would be 49 characters after including the minus sign without a null 
> terminator). If you always want a complete message no matter what the value 
> formatted as %d will be I believe it needs to have a length of 50. The worst 
> that will happen now though because you're using snprintf is you'll drop the 
> trailing "d" or "ed" in the message with very large positive or negative 
> numbers.
> 
> My somewhat sketchy memory of the hpsa driver is that the nr_cmds member of 
> the struct ctlr_info is never more than 1040 (?) anyway. If things are 
> working correctly I don't think it should ever happen but I thought I should 
> point out that msg isn't large enough to contain the complete contents of the 
> longest possible character string.

My knowledge is sketchier than yours.

When I was reading this I was thinking that instead of 1040 the
upper limit was 32.  I got that from hpsa_get_max_perf_mode_cmds().
The only negative number was -1, I think.

But either way, we both agree that 48 is probably safe.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sd: Fix maximum I/O size for BLOCK_PC requests

2015-06-24 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: configurable discard parameters

2015-06-24 Thread Tom Yan
To be honest I don't think this patch should get in as well. First of
all it wouldn't really solve problem for all devices which have
different limits. Secondly I doubt it's related to Deterministic Zero
AT ALL. (For one the SanDisk drive I have shows Deterministic Zero,
still it behaves similarly as the Intel drive.) IHMO adding "switch"
base on characteristics which is uncertain to be / not at all relevant
is the worst thing to do ever. (Yeah I recall how Intel "follows"
HDMI/DP color range spec senselessly again.)

The only patching which would really mean something is to allow user
to configure blocks per range and ranges per command, so that for
users can tune kernel TRIM per device if they really want to, while
leaving the "safe default" intact.

However I am really curious how the drives "blow up" on less blocks
per range. Isn't that even more of a firmware bug than the problem I
have? If they blow up even with single range with less then 65535
blocks addressed, wouldn't they blow up anytime if they do filesystem
TRIM?

On 24 June 2015 at 10:55, Martin K. Petersen  wrote:
>> "Tom" == Tom Yan  writes:
>
> Tom> total untrimmed sectors: (310635 - 1 - 2410) / 32 = 9632 total
> Tom> ranges: (43 GiB * (1024 ^ 3) / 512) / 65535 ~= 1376 average
> Tom> untrimmed sectors per range: (9632 / 1376) = 7 = (65535 % 8)
>
> Every type of drive has its own internal restrictions. Unless the drive
> guarantees zero after TRIM it is perfectly normal for heads, tails or
> random pieces in-between to be left untouched.
>
> I am surprised that the common case of contiguous range entries was not
> handled properly by your drive. Most of them deal with that just fine
> (regardless of their internal granularity and alignment constraints). It
> is normally only the partial block tracking between command invocations
> that causes grief.
>
> In any case. Unless discard_zeroes_data is 1 (and that requires
> guarantees above and beyond what can be discovered via the ATA Command
> Set) all bets are off wrt. dependable behavior.
>
> The code below is an untested proof of concept to see what it would take
> to alleviate your particular issue. I am, however, not at all convinced
> that introducing such a change is worth the risk. I know of at least a
> couple of devices that would blow up as a result of this patch...
>
> --
> Martin K. Petersen  Oracle Linux Engineering
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3131adcc1f87..9c270a303f0b 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2238,8 +2238,24 @@ static unsigned int ata_scsiop_inq_b0(struct 
> ata_scsi_args *args, u8 *rbuf)
>  * with the unmap bit set.
>  */
> if (ata_id_has_trim(args->id)) {
> -   put_unaligned_be64(65535 * 512 / 8, &rbuf[36]);
> -   put_unaligned_be32(1, &rbuf[28]);
> +   /*
> +* If the drive supports reliable zero after trim we set
> +* the granularity to 1 and the blocks per range to
> +* 0x. Otherwise we set set the granularity to 8 and
> +* restrict the blocks per range to 0xfff8. This is done
> +* to accommodate older drives which prefer 4K-alignment.
> +*/
> +
> +   if (ata_id_has_zero_after_trim(args->id) &&
> +   args->dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
> +   put_unaligned_be64(ATA_MAX_TRIM_RANGE * 512 / 8,
> +  &rbuf[36]);
> +   put_unaligned_be32(1, &rbuf[28]);
> +   } else {
> +   put_unaligned_be64(ATA_ALIGNED_TRIM_RANGE * 512 / 8,
> +  &rbuf[36]);
> +   put_unaligned_be32(8, &rbuf[28]);
> +   }
> }
>
> return 0;
> @@ -3168,7 +3184,14 @@ static unsigned int ata_scsi_write_same_xlat(struct 
> ata_queued_cmd *qc)
> goto invalid_fld;
>
> buf = page_address(sg_page(scsi_sglist(scmd)));
> -   size = ata_set_lba_range_entries(buf, 512, block, n_block);
> +
> +   if (ata_id_has_zero_after_trim(dev->id) &&
> +   dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM)
> +   size = ata_set_lba_range_entries(buf, 512, block, n_block,
> +ATA_MAX_TRIM_RANGE);
> +   else
> +   size = ata_set_lba_range_entries(buf, 512, block, n_block,
> +ATA_ALIGNED_TRIM_RANGE);
>
> if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) {
> /* Newer devices support queued TRIM commands */
> diff --git a/include/linux/ata.h b/include/linux/ata.h
> index fed36418dd1c..8a17fa22cdbe 100644
> --- a/include/linux/ata.h
> +++ b/include/linux/ata.h
> @@ -47,6 +47,8 @@ enum {
> ATA_MAX_SECTORS = 256,
> ATA_MAX_SECTORS_L

[PATCH] SCSI-OSD: Delete an unnecessary check before the function call "put_disk"

2015-06-24 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 24 Jun 2015 16:06:21 +0200

The put_disk() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/osd/osd_uld.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 243eab3..e2075522 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -407,9 +407,7 @@ static void __remove(struct device *dev)
 
OSD_INFO("osd_remove %s\n",
 oud->disk ? oud->disk->disk_name : NULL);
-
-   if (oud->disk)
-   put_disk(oud->disk);
+   put_disk(oud->disk);
ida_remove(&osd_minor_ida, oud->minor);
 
kfree(oud);
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] SCSI-OSD: Delete an unnecessary check before the function call "put_disk"

2015-06-24 Thread Johannes Thumshirn
On Wed, Jun 24, 2015 at 04:16:34PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Wed, 24 Jun 2015 16:06:21 +0200
> 
> The put_disk() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/scsi/osd/osd_uld.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index 243eab3..e2075522 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -407,9 +407,7 @@ static void __remove(struct device *dev)
>  
>   OSD_INFO("osd_remove %s\n",
>oud->disk ? oud->disk->disk_name : NULL);
> -
> - if (oud->disk)
> - put_disk(oud->disk);
> + put_disk(oud->disk);
>   ida_remove(&osd_minor_ida, oud->minor);
>  
>   kfree(oud);
> -- 
> 2.4.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn   Storage
jthumsh...@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Greg KH (gre...@linuxfoundation.org)
On Wed, Jun 24, 2015 at 03:25:57PM +0900, Sergey Senozhatsky wrote:
> On (06/24/15 06:10), Seymour, Shane M wrote:
> [..]
> >  
> >  /* The sysfs driver interface. Read-only at the moment */
> > -static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
> > +static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
> >  {
> > -   return snprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
> > +   return sprintf(buf, "%d\n", try_direct_io);
> >  }
> 
> a nitpick,
> 
> per Documentation/filesystems/sysfs.txt
> 
> :
> : - show() should always use scnprintf().
> :

Don't believe everything you read, this change is just fine.

But, you are doing something here that you did not say you were doing in
the changelog, so for that reason, the patch should be redone.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Greg KH (gre...@linuxfoundation.org)
On Wed, Jun 24, 2015 at 03:25:57PM +0900, Sergey Senozhatsky wrote:
> On (06/24/15 06:10), Seymour, Shane M wrote:
> [..]
> >  
> >  /* The sysfs driver interface. Read-only at the moment */
> > -static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
> > +static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
> >  {
> > -   return snprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
> > +   return sprintf(buf, "%d\n", try_direct_io);
> >  }
> 
> a nitpick,
> 
> per Documentation/filesystems/sysfs.txt
> 
> :
> : - show() should always use scnprintf().
> :

That should be rewritten to say, "don't use snprintf(), but scnprintf(),
if you want to.  Otherwise sprintf() should be fine as you obviously are
only returning a single value to userspace"

Or something like that.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Greg KH (gre...@linuxfoundation.org)
On Wed, Jun 24, 2015 at 06:54:35AM +, Seymour, Shane M wrote:
> 
> Convert DRIVER_ATTR macros to DRIVER_ATTR_RO requested by
> Greg KH. Also switched to using scnprintf instead of snprintf
> per Documentation/filesystems/sysfs.txt.
> 
> Suggested-by: Greg Kroah-Hartman 
> Signed-off-by: Shane Seymour 
> ---
> This patch was implemented on top of the previous patch to
> convert to using driver attr groups.
> 
> Changes from v1:
> - switched to scnprintf from sprintf after feedback from Sergey
> Senozhatsky.

Acked-by: Greg Kroah-Hartman 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] SCSI-wd33c93: Deletion of a check before the function call "wd33c93_setup"

2015-06-24 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 24 Jun 2015 17:15:17 +0200

The wd33c93_setup() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/wd33c93.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/wd33c93.c b/drivers/scsi/wd33c93.c
index 9e09da4..e5ed956 100644
--- a/drivers/scsi/wd33c93.c
+++ b/drivers/scsi/wd33c93.c
@@ -1939,7 +1939,7 @@ wd33c93_init(struct Scsi_Host *instance, const 
wd33c93_regs regs,
int val;
char buf[32];
 
-   if (!done_setup && setup_strings)
+   if (!done_setup)
wd33c93_setup(setup_strings);
 
hostdata = (struct WD33C93_hostdata *) instance->hostdata;
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] SCSI-libfc: Delete an unnecessary check before the function call "fc_fcp_ddp_done"

2015-06-24 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 24 Jun 2015 17:50:32 +0200

The fc_fcp_ddp_done() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/libfc/fc_exch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 1b3a094..aad6f48 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -2120,8 +2120,7 @@ static struct fc_seq *fc_exch_seq_send(struct fc_lport 
*lport,
spin_unlock_bh(&ep->ex_lock);
return sp;
 err:
-   if (fsp)
-   fc_fcp_ddp_done(fsp);
+   fc_fcp_ddp_done(fsp);
rc = fc_exch_done_locked(ep);
spin_unlock_bh(&ep->ex_lock);
if (!rc)
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] SCSI: Delete unnecessary checks before the function call "put_device"

2015-06-24 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 24 Jun 2015 19:24:01 +0200

The put_device() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/hosts.c  | 4 +---
 drivers/scsi/scsi_sysfs.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e..84c8565 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -336,9 +336,7 @@ static void scsi_host_dev_release(struct device *dev)
}
 
kfree(shost->shost_data);
-
-   if (parent)
-   put_device(parent);
+   put_device(parent);
kfree(shost);
 }
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 1ac38e7..9ed5391 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -425,9 +425,7 @@ static void scsi_device_dev_release_usercontext(struct 
work_struct *work)
kfree(sdev->vpd_pg80);
kfree(sdev->inquiry);
kfree(sdev);
-
-   if (parent)
-   put_device(parent);
+   put_device(parent);
 }
 
 static void scsi_device_dev_release(struct device *dev)
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] SCSI-fnic: Delete an unnecessary check before the function call "vfree"

2015-06-24 Thread SF Markus Elfring
From: Markus Elfring 
Date: Wed, 24 Jun 2015 20:40:35 +0200

The vfree() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/scsi/fnic/fnic_debugfs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c
index d6498fa..855d382 100644
--- a/drivers/scsi/fnic/fnic_debugfs.c
+++ b/drivers/scsi/fnic/fnic_debugfs.c
@@ -102,9 +102,7 @@ void fnic_debugfs_terminate(void)
 
debugfs_remove(fnic_trace_debugfs_root);
fnic_trace_debugfs_root = NULL;
-
-   if (fc_trc_flag)
-   vfree(fc_trc_flag);
+   vfree(fc_trc_flag);
 }
 
 /*
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: convert to using driver attr groups for sysfs

2015-06-24 Thread Kai Mäkisara (Kolumbus)

> On 23.6.2015, at 11.11, Seymour, Shane M  wrote:
> 
> This patch changes the st driver to use attribute groups so
> driver sysfs files are created automatically. See the
> following for reference:
> 
> http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
> 

Acked-by: Kai Mäkisara 

Thanks,
Kai

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Kai Mäkisara (Kolumbus)

> On 24.6.2015, at 9.54, Seymour, Shane M  wrote:
> 
> 
> Convert DRIVER_ATTR macros to DRIVER_ATTR_RO requested by
> Greg KH. Also switched to using scnprintf instead of snprintf
> per Documentation/filesystems/sysfs.txt.
> 
> Suggested-by: Greg Kroah-Hartman 
> Signed-off-by: Shane Seymour 
> ---
> This patch was implemented on top of the previous patch to
> convert to using driver attr groups.
> 
> Changes from v1:
> - switched to scnprintf from sprintf after feedback from Sergey
> Senozhatsky.

Acked-by: Kai Mäkisara 

Thanks,
Kai

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

2015-06-24 Thread Sergey Senozhatsky
On (06/24/15 08:10), Greg KH  
(gre...@linuxfoundation.org) wrote:
> On Wed, Jun 24, 2015 at 03:25:57PM +0900, Sergey Senozhatsky wrote:
> > On (06/24/15 06:10), Seymour, Shane M wrote:
> > [..]
> > >  
> > >  /* The sysfs driver interface. Read-only at the moment */
> > > -static ssize_t st_try_direct_io_show(struct device_driver *ddp, char 
> > > *buf)
> > > +static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
> > >  {
> > > - return snprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
> > > + return sprintf(buf, "%d\n", try_direct_io);
> > >  }
> > 
> > a nitpick,
> > 
> > per Documentation/filesystems/sysfs.txt
> > 
> > :
> > : - show() should always use scnprintf().
> > :
> 
> That should be rewritten to say, "don't use snprintf(), but scnprintf(),
> if you want to.  Otherwise sprintf() should be fine as you obviously are
> only returning a single value to userspace"
> 

Sure, that was just a nitpick. For '%d' it's totally fine, I agree.
It was more of a 'do we strictly obey the rules' thing.

-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: configurable discard parameters

2015-06-24 Thread Martin K. Petersen
> "Tom" == Tom Yan  writes:

Tom> First of all it wouldn't really solve problem for all devices which
Tom> have different limits.

Only by virtue of us generally aligning on large power of two
boundaries.

The only reason I am entertaining this in the first place is that I have
one drive that behaves in a way similar to yours. And if we can make
things slightly better (but not perfect) for several drives without
causing any regressions then that's worth exploring.

Tom> Secondly I doubt it's related to Deterministic Zero AT ALL. (For
Tom> one the SanDisk drive I have shows Deterministic Zero, still it
Tom> behaves similarly as the Intel drive.)

The drive reporting deterministic zero is not enough. It needs to be
explicitly whitelisted before we report discard_zeroes_data=1.

Tom> The only patching which would really mean something is to allow
Tom> user to configure blocks per range and ranges per command, so that
Tom> for users can tune kernel TRIM per device if they really want to,
Tom> while leaving the "safe default" intact.

I'll think about it.

Tom> However I am really curious how the drives "blow up" on less blocks
Tom> per range. Isn't that even more of a firmware bug than the problem
Tom> I have?

I have several older drives that expect a single contiguous LBA
range. They don't handle multiple discontiguous ranges at all.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html