Re: [PATCH v2] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO
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
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
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
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
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
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
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"
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"
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
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
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
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"
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"
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"
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"
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
> 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
> 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
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
> "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