[PATCH] qedi: Drop cqe response during connection recovery

2018-01-18 Thread Manish Rangankar
We get stuck in the loop when firmware sends a cqe response
during connection recovery.

Signed-off-by: Manish Rangankar 
---
 drivers/scsi/qedi/qedi_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 34a..8808f0d 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -998,7 +998,9 @@ static bool qedi_process_completions(struct qedi_fastpath 
*fp)
 
ret = qedi_queue_cqe(qedi, cqe, fp->sb_id, p);
if (ret)
-   continue;
+   QEDI_WARN(&qedi->dbg_ctx,
+ "Dropping CQE 0x%x for cid=0x%x.\n",
+ que->cq_cons_idx, cqe->cqe_common.conn_id);
 
que->cq_cons_idx++;
if (que->cq_cons_idx == QEDI_CQ_SIZE)
-- 
1.8.3.1



Re: [PATCH v2] scsi: sd: add new match array for cache_type

2018-01-18 Thread weiping zhang
2018-01-19 10:41 GMT+08:00 Martin K. Petersen :
>
> Hi Weiping,
>
>> Add user friendly command strings sd_wce_rcd to enable/disable
>> write&read cache. User can enable both write and read cache by one of
>> the following commands:
>
> I am not entirely sure what the rationale is behind your patch. Is there
> some deficiency in the existing cache control interface that you are
> trying to address? Or is the problem simply that it is poorly
> documented?

Hi Marin,

currently, there are four combinations as following:
"write through", "none", "write back", "write back, no read (daft)"

cache_type can control both write and read cache, but for "write through" and
"write back" we can not know clearly how to control the read cache.

I prefer use words like"w0r1", "w0r0", "w1r1", "w1r0", that "1" means
enable, "0"
means disable. The user know clearly what they are doing when typing
these short words.

Hi James,

> I don't think we can do this.  The output of the cache type in sysfs is
> a user exported ABI.  We'd potentially break it if we add extra stuff.

If so, I'll not change the output, only add new match array.

> --
> Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH v2 hulk-4.1-next 2/7] 9p: ->evict_inode() should kick out ->i_data, not, ->i_mapping

2018-01-18 Thread jiangyiwen
On 2018/1/19 11:46, jiangyiwen wrote:
> mainline inclusion
> from v4.4-rc5
> commit 4ad78628445d26e5e9487b2e8f23274ad7b0f5d3
> category: bugfix
> bugzilla: 4404
> DTS: NA
> CVE: NA
> 补丁作用:对于块设备文件而言,->i_mapping与
> 普通文件不相同,如果文件中的数据来自于块设备
> 文件,则其->i_mapping将指向的是bdevfs inode
> (master inode)的i_data,而->i_data实际是
> empty的,因此在evict_inode的时候不应该释放
> ->i_mapping(bdevfs_inode->i_data)的所有
> pagecache;同时->i_mapping只有在文件被打开
> 的时候才能访问,此时已经是文件被关闭时不应
> 该访问该变量。
> 
> ---
> [ Upstream commit 4ad78628445d26e5e9487b2e8f23274ad7b0f5d3 ]
> 
> For block devices the pagecache is associated with the inode
> on bdevfs, not with the aliasing ones on the mountable filesystems.
> The latter have its own ->i_data empty and ->i_mapping pointing
> to the (unique per major/minor) bdevfs inode.  That guarantees
> cache coherence between all block device inodes with the same
> device number.
> 
> Eviction of an alias inode has no business trying to evict the
> pages belonging to bdevfs one; moreover, ->i_mapping is only
> safe to access when the thing is opened.  At the time of
> ->evict_inode() the victim is definitely *not* opened.  We are
> about to kill the address space embedded into struct inode
> (inode->i_data) and that's what we need to empty of any pages.
> 
> 9p instance tries to empty inode->i_mapping instead, which is
> both unsafe and bogus - if we have several device nodes with
> the same device number in different places, closing one of them
> should not try to empty the (shared) page cache.
> 
> Fortunately, other instances in the tree are OK; they are
> evicting from &inode->i_data instead, as 9p one should.
> 
> Cc: sta...@vger.kernel.org # v2.6.32+, ones prior to 2.6.36 need only half of 
> that
> Reported-by: "Suzuki K. Poulose" 
> Tested-by: "Suzuki K. Poulose" 
> Signed-off-by: Al Viro 
> Signed-off-by: Yiwen Jiang 
> ---
>  fs/9p/vfs_inode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 3d1f365..9b96add 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -451,9 +451,9 @@ void v9fs_evict_inode(struct inode *inode)
>  {
>   struct v9fs_inode *v9inode = V9FS_I(inode);
> 
> - truncate_inode_pages_final(inode->i_mapping);
> + truncate_inode_pages_final(&inode->i_data);
>   clear_inode(inode);
> - filemap_fdatawrite(inode->i_mapping);
> + filemap_fdatawrite(&inode->i_data);
> 
>   v9fs_cache_inode_put_cookie(inode);
>   /* clunk the fid stashed in writeback_fid */
> 
Hi all,

Sorry, It's my fault, please ignore this email.

Thanks,
Yiwen.



[PATCH v2 hulk-4.1-next 2/7] 9p: ->evict_inode() should kick out ->i_data, not, ->i_mapping

2018-01-18 Thread jiangyiwen
mainline inclusion
from v4.4-rc5
commit 4ad78628445d26e5e9487b2e8f23274ad7b0f5d3
category: bugfix
bugzilla: 4404
DTS: NA
CVE: NA
补丁作用:对于块设备文件而言,->i_mapping与
普通文件不相同,如果文件中的数据来自于块设备
文件,则其->i_mapping将指向的是bdevfs inode
(master inode)的i_data,而->i_data实际是
empty的,因此在evict_inode的时候不应该释放
->i_mapping(bdevfs_inode->i_data)的所有
pagecache;同时->i_mapping只有在文件被打开
的时候才能访问,此时已经是文件被关闭时不应
该访问该变量。

---
[ Upstream commit 4ad78628445d26e5e9487b2e8f23274ad7b0f5d3 ]

For block devices the pagecache is associated with the inode
on bdevfs, not with the aliasing ones on the mountable filesystems.
The latter have its own ->i_data empty and ->i_mapping pointing
to the (unique per major/minor) bdevfs inode.  That guarantees
cache coherence between all block device inodes with the same
device number.

Eviction of an alias inode has no business trying to evict the
pages belonging to bdevfs one; moreover, ->i_mapping is only
safe to access when the thing is opened.  At the time of
->evict_inode() the victim is definitely *not* opened.  We are
about to kill the address space embedded into struct inode
(inode->i_data) and that's what we need to empty of any pages.

9p instance tries to empty inode->i_mapping instead, which is
both unsafe and bogus - if we have several device nodes with
the same device number in different places, closing one of them
should not try to empty the (shared) page cache.

Fortunately, other instances in the tree are OK; they are
evicting from &inode->i_data instead, as 9p one should.

Cc: sta...@vger.kernel.org # v2.6.32+, ones prior to 2.6.36 need only half of 
that
Reported-by: "Suzuki K. Poulose" 
Tested-by: "Suzuki K. Poulose" 
Signed-off-by: Al Viro 
Signed-off-by: Yiwen Jiang 
---
 fs/9p/vfs_inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 3d1f365..9b96add 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -451,9 +451,9 @@ void v9fs_evict_inode(struct inode *inode)
 {
struct v9fs_inode *v9inode = V9FS_I(inode);

-   truncate_inode_pages_final(inode->i_mapping);
+   truncate_inode_pages_final(&inode->i_data);
clear_inode(inode);
-   filemap_fdatawrite(inode->i_mapping);
+   filemap_fdatawrite(&inode->i_data);

v9fs_cache_inode_put_cookie(inode);
/* clunk the fid stashed in writeback_fid */
-- 
1.8.3.1




Re: [PATCH-next] ibmvfc: Remove unneeded semicolons

2018-01-18 Thread Christopher Díaz Riveros
El jue, 18-01-2018 a las 21:36 -0500, Martin K. Petersen escribió:
> Christopher,
> 
> > Trivial fix removes unneeded semicolons after switch blocks.
> 
> Applied to 4.16/scsi-queue.
> 

Thank you very much Martin and Tyrel,

Sorry if it's a very tiny trivial patch, but I'm still learning all the
submitting patch procedures and is actually my second patch accepted :)

Regards,
-- 
Christopher Díaz Riveros
Gentoo Linux Developer
GPG Fingerprint: E517 5ECB 8152 98E4 FEBC  2BAA 4DBB D10F 0FDD 2547

signature.asc
Description: This is a digitally signed message part


Re: [PATCH] scsi: fas216: fix sense buffer initialization

2018-01-18 Thread Martin K. Petersen

Arnd,

> While testing with the ARM specific memset() macro removed, I ran
> into a compiler warning that shows an old bug:
>
> drivers/scsi/arm/fas216.c: In function 'fas216_rq_sns_done':
> drivers/scsi/arm/fas216.c:2014:40: error: argument to 'sizeof' in 'memset' 
> call is the same expression as the destination; did you mean to provide an 
> explicit length? [-Werror=sizeof-pointer-memaccess]
>
> It turns out that the definition of the scsi_cmd structure changed back
> in linux-2.6.25, so now we clear only four bytes (sizeof(pointer)) instead
> of 96 (SCSI_SENSE_BUFFERSIZE). I did not check whether we actually need
> to initialize the buffer here, but it's clear that if we do it, we
> should use the correct size.

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH v2] scsi: sd: add new match array for cache_type

2018-01-18 Thread Martin K. Petersen

Hi Weiping,

> Add user friendly command strings sd_wce_rcd to enable/disable
> write&read cache. User can enable both write and read cache by one of
> the following commands:

I am not entirely sure what the rationale is behind your patch. Is there
some deficiency in the existing cache control interface that you are
trying to address? Or is the problem simply that it is poorly
documented?

-- 
Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH-next] ibmvfc: Remove unneeded semicolons

2018-01-18 Thread Martin K. Petersen

Christopher,

> Trivial fix removes unneeded semicolons after switch blocks.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH] [RESEND] megaraid: use ktime_get_real for firmware time

2018-01-18 Thread Martin K. Petersen

Arnd,

> do_gettimeofday() overflows in 2038 on 32-bit architectures and
> is deprecated, so convert this driver to call ktime_get_real()
> directly. This also simplifies the calculation.

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH 0/3] hisi_sas: v2 hw LED support

2018-01-18 Thread Martin K. Petersen

John,

> This patchset includes SGPIO support for driving LEDs for boards
> including a SoC (like hip07) with v2 hw.

Applied to 4.16/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

2018-01-18 Thread Laurent Pinchart
Hi Will,

On Thursday, 18 January 2018 19:05:47 EET Will Deacon wrote:
> On Thu, Jan 18, 2018 at 08:58:08AM -0800, Dan Williams wrote:
> > On Thu, Jan 18, 2018 at 5:18 AM, Will Deacon wrote:
> >> On Thu, Jan 11, 2018 at 05:41:08PM -0800, Dan Williams wrote:
> >>> On Thu, Jan 11, 2018 at 5:19 PM, Linus Torvalds wrote:
>  On Thu, Jan 11, 2018 at 4:46 PM, Dan Williams wrote:
> > This series incorporates Mark Rutland's latest ARM changes and adds
> > the x86 specific implementation of 'ifence_array_ptr'. That ifence
> > based approach is provided as an opt-in fallback, but the default
> > mitigation, '__array_ptr', uses a 'mask' approach that removes
> > conditional branches instructions, and otherwise aims to redirect
> > speculation to use a NULL pointer rather than a user controlled
> > value.
>  
>  Do you have any performance numbers and perhaps example code
>  generation? Is this noticeable? Are there any microbenchmarks showing
>  the difference between lfence use and the masking model?
> >>> 
> >>> I don't have performance numbers, but here's a sample code generation
> >>> from __fcheck_files, where the 'and; lea; and' sequence is portion of
> >>> array_ptr() after the mask generation with 'sbb'.
> >>> 
> >>> fdp = array_ptr(fdt->fd, fd, fdt->max_fds);
> >>>  
> >>>  8e7:   8b 02   mov(%rdx),%eax
> >>>  8e9:   48 39 c7cmp%rax,%rdi
> >>>  8ec:   48 19 c9sbb%rcx,%rcx
> >>>  8ef:   48 8b 42 08 mov0x8(%rdx),%rax
> >>>  8f3:   48 89 femov%rdi,%rsi
> >>>  8f6:   48 21 ceand%rcx,%rsi
> >>>  8f9:   48 8d 04 f0 lea(%rax,%rsi,8),%rax
> >>>  8fd:   48 21 c8and%rcx,%rax
>  
>  Having both seems good for testing, but wouldn't we want to pick one
>  in the end?
> >>> 
> >>> I was thinking we'd keep it as a 'just in case' sort of thing, at
> >>> least until the 'probably safe' assumption of the 'mask' approach has
> >>> more time to settle out.
> >> 
> >> From the arm64 side, the only concern I have (and this actually applies
> >> to our CSDB sequence as well) is the calculation of the array size by
> >> the caller. As Linus mentioned at the end of [1], if the determination
> >> of the size argument is based on a conditional branch, then masking
> >> doesn't help because you bound within the wrong range under speculation.
> >> 
> >> We ran into this when trying to use masking to protect our uaccess
> >> routines where the conditional bound is either KERNEL_DS or USER_DS.
> >> It's possible that a prior conditional set_fs(KERNEL_DS) could defeat
> >> the masking and so we'd need to throw some heavy barriers in set_fs to
> >> make it robust.
> > 
> > At least in the conditional mask case near set_fs() usage the approach
> > we are taking is to use a barrier. I.e. the following guidance from
> > Linus:
> > 
> > "Basically, the rule is trivial: find all 'stac' users, and use address
> > masking if those users already integrate the limit check, and lfence
> > they don't."
> > 
> > ...which translates to narrow the pointer for get_user() and use a
> > barrier  for __get_user().
> 
> Great, that matches my thinking re set_fs but I'm still worried about
> finding all the places where the bound is conditional for other users
> of the macro. Then again, finding the places that need this macro in the
> first place is tough enough so perhaps analysing the bound calculation
> doesn't make it much worse.

It might not now, but if the bound calculation changes later, I'm pretty sure 
we'll forget to update the speculation barrier macro at least in some cases. 
Without the help of static (or possibly dynamic) code analysis I think we're 
bound to reintroduce problems over time, but that's true for finding places 
where the barrier is needed, not just for barrier selection based on bound 
calculation.

-- 
Regards,

Laurent Pinchart



Re: [PATCH-next] ibmvfc: Remove unneeded semicolons

2018-01-18 Thread Tyrel Datwyler
On 01/17/2018 05:38 PM, Christopher Díaz Riveros wrote:
> Trivial fix removes unneeded semicolons after switch blocks.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Christopher Díaz Riveros 
> ---

Resending from the correct email address. :)

Acked-by: Tyrel Datwyler 



Re: [PATCH-next] ibmvfc: Remove unneeded semicolons

2018-01-18 Thread Tyrel Datwyler
On 01/17/2018 05:38 PM, Christopher Díaz Riveros wrote:
> Trivial fix removes unneeded semicolons after switch blocks.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Christopher Díaz Riveros 
> ---

Acked-by: Tyrel Datwyler 


Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

2018-01-18 Thread Will Deacon
On Thu, Jan 18, 2018 at 08:58:08AM -0800, Dan Williams wrote:
> On Thu, Jan 18, 2018 at 5:18 AM, Will Deacon  wrote:
> > On Thu, Jan 11, 2018 at 05:41:08PM -0800, Dan Williams wrote:
> >> On Thu, Jan 11, 2018 at 5:19 PM, Linus Torvalds
> >>  wrote:
> >> > On Thu, Jan 11, 2018 at 4:46 PM, Dan Williams  
> >> > wrote:
> >> >>
> >> >> This series incorporates Mark Rutland's latest ARM changes and adds
> >> >> the x86 specific implementation of 'ifence_array_ptr'. That ifence
> >> >> based approach is provided as an opt-in fallback, but the default
> >> >> mitigation, '__array_ptr', uses a 'mask' approach that removes
> >> >> conditional branches instructions, and otherwise aims to redirect
> >> >> speculation to use a NULL pointer rather than a user controlled value.
> >> >
> >> > Do you have any performance numbers and perhaps example code
> >> > generation? Is this noticeable? Are there any microbenchmarks showing
> >> > the difference between lfence use and the masking model?
> >>
> >> I don't have performance numbers, but here's a sample code generation
> >> from __fcheck_files, where the 'and; lea; and' sequence is portion of
> >> array_ptr() after the mask generation with 'sbb'.
> >>
> >> fdp = array_ptr(fdt->fd, fd, fdt->max_fds);
> >>  8e7:   8b 02   mov(%rdx),%eax
> >>  8e9:   48 39 c7cmp%rax,%rdi
> >>  8ec:   48 19 c9sbb%rcx,%rcx
> >>  8ef:   48 8b 42 08 mov0x8(%rdx),%rax
> >>  8f3:   48 89 femov%rdi,%rsi
> >>  8f6:   48 21 ceand%rcx,%rsi
> >>  8f9:   48 8d 04 f0 lea(%rax,%rsi,8),%rax
> >>  8fd:   48 21 c8and%rcx,%rax
> >>
> >>
> >> > Having both seems good for testing, but wouldn't we want to pick one in 
> >> > the end?
> >>
> >> I was thinking we'd keep it as a 'just in case' sort of thing, at
> >> least until the 'probably safe' assumption of the 'mask' approach has
> >> more time to settle out.
> >
> > From the arm64 side, the only concern I have (and this actually applies to
> > our CSDB sequence as well) is the calculation of the array size by the
> > caller. As Linus mentioned at the end of [1], if the determination of the
> > size argument is based on a conditional branch, then masking doesn't help
> > because you bound within the wrong range under speculation.
> >
> > We ran into this when trying to use masking to protect our uaccess routines
> > where the conditional bound is either KERNEL_DS or USER_DS. It's possible
> > that a prior conditional set_fs(KERNEL_DS) could defeat the masking and so
> > we'd need to throw some heavy barriers in set_fs to make it robust.
> 
> At least in the conditional mask case near set_fs() usage the approach
> we are taking is to use a barrier. I.e. the following guidance from
> Linus:
> 
> "Basically, the rule is trivial: find all 'stac' users, and use address
> masking if those users already integrate the limit check, and lfence
> they don't."
> 
> ...which translates to narrow the pointer for get_user() and use a
> barrier  for __get_user().

Great, that matches my thinking re set_fs but I'm still worried about
finding all the places where the bound is conditional for other users
of the macro. Then again, finding the places that need this macro in the
first place is tough enough so perhaps analysing the bound calculation
doesn't make it much worse.

Will


Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

2018-01-18 Thread Dan Williams
On Thu, Jan 18, 2018 at 5:18 AM, Will Deacon  wrote:
> Hi Dan, Linus,
>
> On Thu, Jan 11, 2018 at 05:41:08PM -0800, Dan Williams wrote:
>> On Thu, Jan 11, 2018 at 5:19 PM, Linus Torvalds
>>  wrote:
>> > On Thu, Jan 11, 2018 at 4:46 PM, Dan Williams  
>> > wrote:
>> >>
>> >> This series incorporates Mark Rutland's latest ARM changes and adds
>> >> the x86 specific implementation of 'ifence_array_ptr'. That ifence
>> >> based approach is provided as an opt-in fallback, but the default
>> >> mitigation, '__array_ptr', uses a 'mask' approach that removes
>> >> conditional branches instructions, and otherwise aims to redirect
>> >> speculation to use a NULL pointer rather than a user controlled value.
>> >
>> > Do you have any performance numbers and perhaps example code
>> > generation? Is this noticeable? Are there any microbenchmarks showing
>> > the difference between lfence use and the masking model?
>>
>> I don't have performance numbers, but here's a sample code generation
>> from __fcheck_files, where the 'and; lea; and' sequence is portion of
>> array_ptr() after the mask generation with 'sbb'.
>>
>> fdp = array_ptr(fdt->fd, fd, fdt->max_fds);
>>  8e7:   8b 02   mov(%rdx),%eax
>>  8e9:   48 39 c7cmp%rax,%rdi
>>  8ec:   48 19 c9sbb%rcx,%rcx
>>  8ef:   48 8b 42 08 mov0x8(%rdx),%rax
>>  8f3:   48 89 femov%rdi,%rsi
>>  8f6:   48 21 ceand%rcx,%rsi
>>  8f9:   48 8d 04 f0 lea(%rax,%rsi,8),%rax
>>  8fd:   48 21 c8and%rcx,%rax
>>
>>
>> > Having both seems good for testing, but wouldn't we want to pick one in 
>> > the end?
>>
>> I was thinking we'd keep it as a 'just in case' sort of thing, at
>> least until the 'probably safe' assumption of the 'mask' approach has
>> more time to settle out.
>
> From the arm64 side, the only concern I have (and this actually applies to
> our CSDB sequence as well) is the calculation of the array size by the
> caller. As Linus mentioned at the end of [1], if the determination of the
> size argument is based on a conditional branch, then masking doesn't help
> because you bound within the wrong range under speculation.
>
> We ran into this when trying to use masking to protect our uaccess routines
> where the conditional bound is either KERNEL_DS or USER_DS. It's possible
> that a prior conditional set_fs(KERNEL_DS) could defeat the masking and so
> we'd need to throw some heavy barriers in set_fs to make it robust.

At least in the conditional mask case near set_fs() usage the approach
we are taking is to use a barrier. I.e. the following guidance from
Linus:

"Basically, the rule is trivial: find all 'stac' users, and use address
masking if those users already integrate the limit check, and lfence
they don't."

...which translates to narrow the pointer for get_user() and use a
barrier  for __get_user().


Re: [PATCH] sd: succeed check_event if device is not removable

2018-01-18 Thread James Bottomley
On Thu, 2018-01-18 at 17:22 +0100, Jack Wang wrote:
> From: Jack Wang 
> 
> The check_events interface was added for check if device changes,
> mainly for device is removable eg. CDROM
> 
> In sd_open, it checks if device is removable then check_disk_change.
> 
> when the device is not removable, we can simple succeeds the call
> without send TUR.
> 
> Signed-off-by: Jack Wang 
> ---
>  drivers/scsi/sd.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index ab75ebd..773ce81 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1576,6 +1576,10 @@ static unsigned int sd_check_events(struct
> gendisk *disk, unsigned int clearing)
>   sdp = sdkp->device;
>   SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp,
> "sd_check_events\n"));
>  
> + if (!sdp->removable) {
> + retval = 0;
> + goto out;
> + }

This looks like very much the wrong place to fix whatever problem
you're seeing is.  We could simply avoid setting up the events work
function in genhd.c:device_add_disk(), which would be way more
efficient.  However, I think some devices do require being probed
occasionally with a TUR because its the only way SCSI gets AENs (not
that we make much use of them).

So first of all, what's the actual problem?

James



Re: [PATCH v2] scsi: sd: add new match array for cache_type

2018-01-18 Thread James Bottomley
On Thu, 2018-01-18 at 22:19 +0800, weiping zhang wrote:
> - return sprintf(buf, "%s\n", sd_cache_types[ct]);
> + return sprintf(buf, "%s\n%s\nwrite:%s, read:%s\n",
> sd_cache_types[ct],
> + sd_wce_rcd[ct], sdkp->WCE ? "on" : "off",
> + sdkp->RCD ? "off" : "on");

I don't think we can do this.  The output of the cache type in sysfs is
a user exported ABI.  We'd potentially break it if we add extra stuff.

James



[PATCH] sd: succeed check_event if device is not removable

2018-01-18 Thread Jack Wang
From: Jack Wang 

The check_events interface was added for check if device changes,
mainly for device is removable eg. CDROM

In sd_open, it checks if device is removable then check_disk_change.

when the device is not removable, we can simple succeeds the call
without send TUR.

Signed-off-by: Jack Wang 
---
 drivers/scsi/sd.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index ab75ebd..773ce81 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1576,6 +1576,10 @@ static unsigned int sd_check_events(struct gendisk 
*disk, unsigned int clearing)
sdp = sdkp->device;
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_check_events\n"));
 
+   if (!sdp->removable) {
+   retval = 0;
+   goto out;
+   }
/*
 * If the device is offline, don't send any commands - just pretend as
 * if the command failed.  If the device ever comes back online, we
-- 
2.7.4



RE: [PATCH] [RESEND] megaraid: use ktime_get_real for firmware time

2018-01-18 Thread Sumit Saxena
-Original Message-
From: Arnd Bergmann [mailto:a...@arndb.de]
Sent: Wednesday, January 17, 2018 8:19 PM
To: Kashyap Desai; Sumit Saxena; Shivasharan S; James E.J. Bottomley;
Martin K. Petersen
Cc: Arnd Bergmann; Tomas Henzl; Hannes Reinecke;
megaraidlinux@broadcom.com; linux-scsi@vger.kernel.org;
linux-ker...@vger.kernel.org
Subject: [PATCH] [RESEND] megaraid: use ktime_get_real for firmware time

do_gettimeofday() overflows in 2038 on 32-bit architectures and is
deprecated, so convert this driver to call ktime_get_real() directly. This
also simplifies the calculation.

Signed-off-by: Arnd Bergmann 
---
Sent originally in Nov 2017, no comments. Please apply
---
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 0a85f3c48ef6..97fae28c8374 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -983,7 +983,7 @@ megasas_ioc_init_fusion(struct megasas_instance
*instance)
MFI_CAPABILITIES *drv_ops;
u32 scratch_pad_2;
unsigned long flags;
-   struct timeval tv;
+   ktime_t time;
bool cur_fw_64bit_dma_capable;

fusion = instance->ctrl_context;
@@ -1042,10 +1042,9 @@ megasas_ioc_init_fusion(struct megasas_instance
*instance)
IOCInitMessage->HostMSIxVectors = instance->msix_vectors;
IOCInitMessage->HostPageSize = MR_DEFAULT_NVME_PAGE_SHIFT;

-   do_gettimeofday(&tv);
+   time = ktime_get_real();
/* Convert to milliseconds as per FW requirement */
-   IOCInitMessage->TimeStamp = cpu_to_le64((tv.tv_sec * 1000) +
-   (tv.tv_usec / 1000));
+   IOCInitMessage->TimeStamp = cpu_to_le64(ktime_to_ms(time));

init_frame = (struct megasas_init_frame *)cmd->frame;
memset(init_frame, 0, IOC_INIT_FRAME_SIZE);
Looks good to me.

Acked-by: Sumit Saxena 

--
2.9.0


[PATCH v2] scsi: sd: add new match array for cache_type

2018-01-18 Thread weiping zhang
Add user friendly command strings sd_wce_rcd to enable/disable
write&read cache. User can enable both write and read cache by one of
the following commands:

echo 10 > cache_type
echo "write back" > cache_type

wce rcd write_cache read_cache
0   0   off on
0   1   off off
1   0   on  on
1   1   on  off

When execute "cat cache_type", it will show more detail info like following:
write back
10
write:on, read:on

Signed-off-by: weiping zhang 
---
 drivers/scsi/sd.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a028ab3..722e440 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -139,6 +139,15 @@ static const char *sd_cache_types[] = {
"write back, no read (daft)"
 };
 
+/*
+ * wce rcd write_cache read_cache
+ * 0   0   off on
+ * 0   1   off off
+ * 1   0   on  on
+ * 1   1   on  off
+ */
+static const char * const sd_wce_rcd[] = {"00", "01", "10", "11"};
+
 static void sd_set_flush_flag(struct scsi_disk *sdkp)
 {
bool wc = false, fua = false;
@@ -180,8 +189,11 @@ cache_type_store(struct device *dev, struct 
device_attribute *attr,
}
 
ct = sysfs_match_string(sd_cache_types, buf);
-   if (ct < 0)
-   return -EINVAL;
+   if (ct < 0) {
+   ct = sysfs_match_string(sd_wce_rcd, buf);
+   if (ct < 0)
+   return -EINVAL;
+   }
 
rcd = ct & 0x01 ? 1 : 0;
wce = (ct & 0x02) && !sdkp->write_prot ? 1 : 0;
@@ -282,7 +294,9 @@ cache_type_show(struct device *dev, struct device_attribute 
*attr, char *buf)
struct scsi_disk *sdkp = to_scsi_disk(dev);
int ct = sdkp->RCD + 2*sdkp->WCE;
 
-   return sprintf(buf, "%s\n", sd_cache_types[ct]);
+   return sprintf(buf, "%s\n%s\nwrite:%s, read:%s\n", sd_cache_types[ct],
+   sd_wce_rcd[ct], sdkp->WCE ? "on" : "off",
+   sdkp->RCD ? "off" : "on");
 }
 static DEVICE_ATTR_RW(cache_type);
 
-- 
2.9.4



Re: [PATCH] scsi: handle ABORTED_COMMAND on Fujitsu ETERNUS

2018-01-18 Thread James Bottomley
On Thu, 2018-01-18 at 11:17 +0100, Hannes Reinecke wrote:
> On 01/18/2018 03:43 AM, Martin K. Petersen wrote:
> > 
> > 
> > Martin,
> > 
> > > 
> > > You'd like to spend a precious BLIST bit for this single device
> > > which uses vendor-specific ASC/Q?
> > 
> > I really don't want string comparisons in the regular code paths.
> > Also not a fan of vendor-specific ASCs. But if you must use them,
> > please add a flag and trigger on that.
> > 
> > Since this is a bit of an unusual check condition combo, we could
> > entertain whether we should simply always ADD_TO_MLQUEUE on
> > 0xb/0xc1/0x1. I wonder what would break?
> > 
> That's the usual problem I have with vendor-specific sense codes.
> In general the risk is quite low of different vendors actually using
> the same code; I guess we can get away with just adding a debug
> message here and enable it without any vendor check.
> Then we can reconsider the whole thing once we notice several vendors
> using the same sense codes.

Murphy's law says that if we rely on Vendors to chose non-overlapping
numbers we'll end up with a clash fairly quickly ...

Can't we find a way of doing this in the device_handler?  That way we
can use vendor specific codes only when we know who the vendor is?

James



Re: [PATCH] [RESEND] scsi: ips: fix firmware timestamps for 32-bit

2018-01-18 Thread Arnd Bergmann
On Thu, Jan 18, 2018 at 10:35 AM, Finn Thain  wrote:
> On Wed, 17 Jan 2018, Arnd Bergmann wrote:
>
>> do_gettimeofday() is deprecated since it will stop working in 2038 on
>> 32-bit platforms. The firmware interface here actually supports times
>> until year 25500, so we should use longer timestamps.
>>
>
> I think that reasoning is flawed. If the firmware supports large year
> values, then fine. The firmware interface is another story.
>
> If you are simply trying to get rid of the old API, I think you should
> just say that.

It's both: I have no reason to believe that the firmware breaks in
2038. We can't know the range of the supported centuries, so it
could break in e.g. 2099, , or 25599 (I should have written
that last number instead of 25500), but we know what the interface
supports, and if the firmware breaks earlier, then it can be fixed
while keeping that interface.

In particular, 64-bit architectures work fine already for as long as
the firmware supports, it's only 32-bit architectures that break
early.

I'll try to explain that better in the changelog.

>>  
>> //
>>  static void
>> -ips_fix_ffdc_time(ips_ha_t * ha, ips_scb_t * scb, time_t current_time)
>> +ips_fix_ffdc_time(ips_ha_t * ha, ips_scb_t * scb, time64_t current_time)
>
> Does this conversion assume that current_time is always positive? I don't
> have a problem with that (the existing code seems to fail otherwise) but
> maybe it should be mentioned in the commit log.

current_time can never be negative, the kernel probably won't even boot
if that were the case, as too many things rely on a positive time. The resulting
numbers in ips_fix_ffdc_time() shouldn't change as far as I can tell (except the
bug you found below).

>> - year = IPS_EPOCH_YEAR;
>> - while (days < 0 || days >= year_lengths[yleap = 
>> IPS_IS_LEAP_YEAR(year)]) {
>> - int newy;
>> -
>> - newy = year + (days / IPS_DAYS_NORMAL_YEAR);
>> - if (days < 0)
>> - --newy;
>> - days -= (newy - year) * IPS_DAYS_NORMAL_YEAR +
>> - IPS_NUM_LEAP_YEARS_THROUGH(newy - 1) -
>> - IPS_NUM_LEAP_YEARS_THROUGH(year - 1);
>
> These IPS_ time macros are all unused after this patch except
> IPS_SECS_8HOURS so the definitions should probably be removed.

Ok, will do.

>> - year = newy;
>> - }
>> -
>> - scb->cmd.ffdc.yearH = year / 100;
>> - scb->cmd.ffdc.yearL = year % 100;
>> -
>> - for (i = 0; days >= month_lengths[i][yleap]; ++i)
>> - days -= month_lengths[i][yleap];
>> + time64_to_tm(current_time, 0, &tm);
>>
>> - scb->cmd.ffdc.month = i + 1;
>> - scb->cmd.ffdc.day = days + 1;
>> + scb->cmd.ffdc.hour   = tm.tm_hour;
>> + scb->cmd.ffdc.minute = tm.tm_min;
>> + scb->cmd.ffdc.second = tm.tm_sec;
>> + scb->cmd.ffdc.yearH  = tm.tm_year / 100 + 1900;
>
> That looks wrong to me. If tm_year is in units of years not centuries,
> shouldn't that be,

Good catch, thanks for the review!

  Arnd


Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

2018-01-18 Thread Will Deacon
Hi Dan, Linus,

On Thu, Jan 11, 2018 at 05:41:08PM -0800, Dan Williams wrote:
> On Thu, Jan 11, 2018 at 5:19 PM, Linus Torvalds
>  wrote:
> > On Thu, Jan 11, 2018 at 4:46 PM, Dan Williams  
> > wrote:
> >>
> >> This series incorporates Mark Rutland's latest ARM changes and adds
> >> the x86 specific implementation of 'ifence_array_ptr'. That ifence
> >> based approach is provided as an opt-in fallback, but the default
> >> mitigation, '__array_ptr', uses a 'mask' approach that removes
> >> conditional branches instructions, and otherwise aims to redirect
> >> speculation to use a NULL pointer rather than a user controlled value.
> >
> > Do you have any performance numbers and perhaps example code
> > generation? Is this noticeable? Are there any microbenchmarks showing
> > the difference between lfence use and the masking model?
> 
> I don't have performance numbers, but here's a sample code generation
> from __fcheck_files, where the 'and; lea; and' sequence is portion of
> array_ptr() after the mask generation with 'sbb'.
> 
> fdp = array_ptr(fdt->fd, fd, fdt->max_fds);
>  8e7:   8b 02   mov(%rdx),%eax
>  8e9:   48 39 c7cmp%rax,%rdi
>  8ec:   48 19 c9sbb%rcx,%rcx
>  8ef:   48 8b 42 08 mov0x8(%rdx),%rax
>  8f3:   48 89 femov%rdi,%rsi
>  8f6:   48 21 ceand%rcx,%rsi
>  8f9:   48 8d 04 f0 lea(%rax,%rsi,8),%rax
>  8fd:   48 21 c8and%rcx,%rax
> 
> 
> > Having both seems good for testing, but wouldn't we want to pick one in the 
> > end?
> 
> I was thinking we'd keep it as a 'just in case' sort of thing, at
> least until the 'probably safe' assumption of the 'mask' approach has
> more time to settle out.

>From the arm64 side, the only concern I have (and this actually applies to
our CSDB sequence as well) is the calculation of the array size by the
caller. As Linus mentioned at the end of [1], if the determination of the
size argument is based on a conditional branch, then masking doesn't help
because you bound within the wrong range under speculation.

We ran into this when trying to use masking to protect our uaccess routines
where the conditional bound is either KERNEL_DS or USER_DS. It's possible
that a prior conditional set_fs(KERNEL_DS) could defeat the masking and so
we'd need to throw some heavy barriers in set_fs to make it robust.

The question then is whether or not we're likely to run into this elsewhere,
and I don't have a good feel for that.

Will

[1] 
http://lkml.kernel.org/r/CA+55aFz0tsreoa=5ud2nofcpng-dizlbht9wu9asyhplfjd...@mail.gmail.com


[PATCH] scsi: fas216: fix sense buffer initialization

2018-01-18 Thread Arnd Bergmann
While testing with the ARM specific memset() macro removed, I ran
into a compiler warning that shows an old bug:

drivers/scsi/arm/fas216.c: In function 'fas216_rq_sns_done':
drivers/scsi/arm/fas216.c:2014:40: error: argument to 'sizeof' in 'memset' call 
is the same expression as the destination; did you mean to provide an explicit 
length? [-Werror=sizeof-pointer-memaccess]

It turns out that the definition of the scsi_cmd structure changed back
in linux-2.6.25, so now we clear only four bytes (sizeof(pointer)) instead
of 96 (SCSI_SENSE_BUFFERSIZE). I did not check whether we actually need
to initialize the buffer here, but it's clear that if we do it, we
should use the correct size.

Fixes: de25deb18016 ("[SCSI] use dynamically allocated sense buffer")
Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/arm/fas216.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index f4775ca70bab..27bda2b05de6 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2011,7 +2011,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct 
scsi_cmnd *SCpnt,
 * have valid data in the sense buffer that could
 * confuse the higher levels.
 */
-   memset(SCpnt->sense_buffer, 0, sizeof(SCpnt->sense_buffer));
+   memset(SCpnt->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
 //printk("scsi%d.%c: sense buffer: ", info->host->host_no, '0' + 
SCpnt->device->id);
 //{ int i; for (i = 0; i < 32; i++) printk("%02x ", SCpnt->sense_buffer[i]); 
printk("\n"); }
/*
-- 
2.9.0



RE: A qla2xxx commit cause Linux no response, has not fixed in lastest version 4.15-rc6

2018-01-18 Thread Changlimin
Hi Himanshu,
  Today I reproduced the issue in my server.
  First, I compiled kernel 4.15-rc6 (make localmodconfig; make; make 
modules_install; make install), then start the kernel with parameter 
modprobe.blacklist=qla2xxx.
  Second,  tail -f /var/log/syslog
  Third,  modprobe qla2xxx ql2xextended_error_logging=0x1e40 , the log is 
syslog-1e40.txt
  The syslog-7fff is got when modprobe qla2xxx 
ql2xextended_error_logging=0x7fff

  BTW, I haven't load driver from 4.9.x to kernel 4.15-rc6. 
  When I checkout kernel commit 726b85487067d7f5b23495bc33c484b8517c4074, all 
kernel code is 4.9.x.

Regards
Chang Limin

-Original Message-
From: Madhani, Himanshu [mailto:himanshu.madh...@cavium.com]
Sent: Thursday, January 18, 2018 2:26 AM
To: changlimin (Cloud)
Cc: Nicholas A. Bellinger; Tran, Quinn; jifuliang (Cloud); zhangguanghui 
(Cloud); zhangzijian (Cloud); target-devel; linux-scsi
Subject: Re: A qla2xxx commit cause Linux no response, has not fixed in lastest 
version 4.15-rc6

Hi Chang, 

> On Jan 15, 2018, at 10:49 PM, Changlimin  wrote:
> 
> Hi Himanshu,
>   This is my progress.
>   First, I compiled 4.15-rc6, I found linux hang when booting, the stack 
> showed something wrong in qla2xxx driver.

Can you provide me detail steps of how you compiled 4.15-rc6. Also provide me 
details of how you are loading driver and also provide complete log file.

I do not see how you will be able to load driver which is from 4.9.x when you 
compile fresh 4.15.0-rc6. 

Just FYI, I build test system with 8G/16G/32G adapter with 4.15.0-rc6 kernel 
and I am not able to see hang that you are describing. 

# uname -r
4.15.0-rc6+

# modprobe qla2xxx

# fcc.sh
FC HBAs:
HBA   Port NamePort ID   State Device
host3 21:00:00:24:ff:7e:f5:80  01:0d:00  OnlineQLE2742 FW:v8.05.63 
DVR:v10.00.00.04-k
host4 21:00:00:24:ff:7e:f5:81  01:0e:00  OnlineQLE2742 FW:v8.05.63 
DVR:v10.00.00.04-k
host5 21:00:00:0e:1e:12:e9:a0  01:06:00  OnlineQLE8362 FW:v8.03.06 
DVR:v10.00.00.04-k
host6 21:00:00:0e:1e:12:e9:a1  01:14:00  OnlineQLE8362 FW:v8.03.06 
DVR:v10.00.00.04-k
host7 21:00:00:24:ff:46:0a:5c  01:0d:00  OnlineQLE2562 FW:v8.03.00 
DVR:v10.00.00.04-k
host8 21:00:00:24:ff:46:0a:5d  01:15:00  OnlineQLE2562 FW:v8.03.00 
DVR:v10.00.00.04-k

# modinfo qla2xxx | more

filename:   /lib/modules/4.15.0-rc6+/kernel/drivers/scsi/qla2xxx/qla2xxx.ko
firmware:   ql2500_fw.bin
firmware:   ql2400_fw.bin
firmware:   ql2322_fw.bin
firmware:   ql2300_fw.bin
firmware:   ql2200_fw.bin
firmware:   ql2100_fw.bin
version:10.00.00.04-k
license:GPL
description:QLogic Fibre Channel HBA Driver
author: QLogic Corporation
srcversion: 6CBCF1372A7756690E83CC3


>   Second, I want to find which commit introduced the issue. So I tried many 
> times via git bisect to linux kernel.
>   Finally, I found the commit 726b85487067d7f5b23495bc33c484b8517c4074 
> introduced the issue. The attached log is related to this commit.
>   Also ubuntu kernel has this issue: 
>   
> https://launchpad.net/ubuntu/+archive/primary/+files/linux-image-4.13.
> 0-25-generic_4.13.0-25.29_amd64.deb
>   
> https://launchpad.net/ubuntu/+archive/primary/+files/linux-image-extra
> -4.13.0-25-generic_4.13.0-25.29_amd64.deb
> 
> Regards
> Chang Limin
> 
> -Original Message-
> From: Madhani, Himanshu [mailto:himanshu.madh...@cavium.com]
> Sent: Tuesday, January 16, 2018 12:59 PM
> To: changlimin (Cloud)
> Cc: Nicholas A. Bellinger; Tran, Quinn; jifuliang (Cloud); 
> zhangguanghui (Cloud); zhangzijian (Cloud); target-devel; linux-scsi
> Subject: Re: A qla2xxx commit cause Linux no response, has not fixed 
> in lastest version 4.15-rc6
> 
> Hi Chang,
> 
>> On Jan 15, 2018, at 4:27 PM, Changlimin  wrote:
>> 
>> Hi Himanshu,
>> The issue is: When insmod the qla2xxx.ko from 4.15-rc6, linux hang.
> 
> From the log file attached. I see that you are trying to load driver from 
> 4.9.x in 4.15.0-rc6. 
> 
> [  279.898704] qla2xxx [:00:00.0]-0005: : QLogic Fibre Channel HBA 
> Driver: 8.07.00.38-k-debug.
> 
> 4.15.0-rc6 had driver version 10.00.00.02-k. Would you check if you have all 
> the driver changes pulled in with kernel 4.15.0-rc6.
> 
>> I have git bisect the commits. 
>> The issue was introduced in commit: 726b85487067d7f5b23495bc33c484b8517c4074 
>> qla2xxx: Add framework for async fabric discovery.
>> The previous commit is good: 5d964837c6a743193c63c8912f98834c7457ba5c 
>> qla2xxx: Track I-T nexus as single fc_port struct .
>> 
>> Regards
>> Chang Limin
>> 
>> -Original Message-
>> From: Madhani, Himanshu [mailto:himanshu.madh...@cavium.com]
>> Sent: Tuesday, January 16, 2018 12:58 AM
>> To: Nicholas A. Bellinger
>> Cc: changlimin (Cloud); Tran, Quinn; jifuliang (Cloud); zhangguanghui 
>> (Cloud); zhangzijian (Cloud); target-devel; linux-scsi
>> Subject: Re: A qla2xxx commit cause Linux no response, has not fixed 
>> in lastest version 4.

Re: [PATCH] scsi: handle ABORTED_COMMAND on Fujitsu ETERNUS

2018-01-18 Thread Hannes Reinecke
On 01/18/2018 03:43 AM, Martin K. Petersen wrote:
> 
> Martin,
> 
>> You'd like to spend a precious BLIST bit for this single device which
>> uses vendor-specific ASC/Q?
> 
> I really don't want string comparisons in the regular code paths. Also
> not a fan of vendor-specific ASCs. But if you must use them, please add
> a flag and trigger on that.
> 
> Since this is a bit of an unusual check condition combo, we could
> entertain whether we should simply always ADD_TO_MLQUEUE on
> 0xb/0xc1/0x1. I wonder what would break?
> 
That's the usual problem I have with vendor-specific sense codes.
In general the risk is quite low of different vendors actually using the
same code; I guess we can get away with just adding a debug message here
and enable it without any vendor check.
Then we can reconsider the whole thing once we notice several vendors
using the same sense codes.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
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)


Re: [PATCH] [RESEND] scsi: ips: fix firmware timestamps for 32-bit

2018-01-18 Thread Finn Thain
On Wed, 17 Jan 2018, Arnd Bergmann wrote:

> do_gettimeofday() is deprecated since it will stop working in 2038 on
> 32-bit platforms. The firmware interface here actually supports times
> until year 25500, so we should use longer timestamps.
> 

I think that reasoning is flawed. If the firmware supports large year 
values, then fine. The firmware interface is another story.

If you are simply trying to get rid of the old API, I think you should 
just say that.

> Using ktime_get_real_seconds() to get a 64-bit seconds value
> and time64_to_tm() to convert it into the right format also has
> the advantage of greatly simplifying the time management code.
> 
> Signed-off-by: Arnd Bergmann 
> ---
> Submitted originally in November 2017. The aacr...@adaptec.com
> apparently bounced. Trying again now with a few people on Cc
> that previously reviewed patches to this driver.
> ---
>  drivers/scsi/ips.c | 78 
> +++---
>  drivers/scsi/ips.h |  2 +-
>  2 files changed, 17 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index 67621308eb9c..887843a465e1 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -293,7 +293,7 @@ static void ips_freescb(ips_ha_t *, ips_scb_t *);
>  static void ips_setup_funclist(ips_ha_t *);
>  static void ips_statinit(ips_ha_t *);
>  static void ips_statinit_memio(ips_ha_t *);
> -static void ips_fix_ffdc_time(ips_ha_t *, ips_scb_t *, time_t);
> +static void ips_fix_ffdc_time(ips_ha_t *, ips_scb_t *, time64_t);
>  static void ips_ffdc_reset(ips_ha_t *, int);
>  static void ips_ffdc_time(ips_ha_t *);
>  static uint32_t ips_statupd_copperhead(ips_ha_t *);
> @@ -989,10 +989,7 @@ static int __ips_eh_reset(struct scsi_cmnd *SC)
>  
>   /* FFDC */
>   if (le32_to_cpu(ha->subsys->param[3]) & 0x30) {
> - struct timeval tv;
> -
> - do_gettimeofday(&tv);
> - ha->last_ffdc = tv.tv_sec;
> + ha->last_ffdc = ktime_get_real_seconds();
>   ha->reset_count++;
>   ips_ffdc_reset(ha, IPS_INTR_IORL);
>   }
> @@ -2396,7 +2393,6 @@ static int
>  ips_hainit(ips_ha_t * ha)
>  {
>   int i;
> - struct timeval tv;
>  
>   METHOD_TRACE("ips_hainit", 1);
>  
> @@ -2411,8 +2407,7 @@ ips_hainit(ips_ha_t * ha)
>  
>   /* Send FFDC */
>   ha->reset_count = 1;
> - do_gettimeofday(&tv);
> - ha->last_ffdc = tv.tv_sec;
> + ha->last_ffdc = ktime_get_real_seconds();
>   ips_ffdc_reset(ha, IPS_INTR_IORL);
>  
>   if (!ips_read_config(ha, IPS_INTR_IORL)) {
> @@ -2552,12 +2547,9 @@ ips_next(ips_ha_t * ha, int intr)
>  
>   if ((ha->subsys->param[3] & 0x30)
>   && (ha->scb_activelist.count == 0)) {
> - struct timeval tv;
> -
> - do_gettimeofday(&tv);
> -
> - if (tv.tv_sec - ha->last_ffdc > IPS_SECS_8HOURS) {
> - ha->last_ffdc = tv.tv_sec;
> + time64_t now = ktime_get_real_seconds();
> + if (now - ha->last_ffdc > IPS_SECS_8HOURS) {
> + ha->last_ffdc = now;
>   ips_ffdc_time(ha);
>   }
>   }
> @@ -5992,59 +5984,21 @@ ips_ffdc_time(ips_ha_t * ha)
>  /*  
> */
>  
> //
>  static void
> -ips_fix_ffdc_time(ips_ha_t * ha, ips_scb_t * scb, time_t current_time)
> +ips_fix_ffdc_time(ips_ha_t * ha, ips_scb_t * scb, time64_t current_time)

Does this conversion assume that current_time is always positive? I don't 
have a problem with that (the existing code seems to fail otherwise) but 
maybe it should be mentioned in the commit log.

>  {
> - long days;
> - long rem;
> - int i;
> - int year;
> - int yleap;
> - int year_lengths[2] = { IPS_DAYS_NORMAL_YEAR, IPS_DAYS_LEAP_YEAR };
> - int month_lengths[12][2] = { {31, 31},
> - {28, 29},
> - {31, 31},
> - {30, 30},
> - {31, 31},
> - {30, 30},
> - {31, 31},
> - {31, 31},
> - {30, 30},
> - {31, 31},
> - {30, 30},
> - {31, 31}
> - };
> + struct tm tm;
>  
>   METHOD_TRACE("ips_fix_ffdc_time", 1);
>  
> - days = current_time / IPS_SECS_DAY;
> - rem = current_time % IPS_SECS_DAY;
> -
> - scb->cmd.ffdc.hour = (rem / IPS_SECS_HOUR);
> - rem = rem % IPS_SECS_HOUR;
> - scb->cmd.ffdc.minute = (rem / IPS_SECS_MIN);
> - scb->cmd.ffdc.second = (rem % IPS_SECS_MIN);
> -
> - year = IPS_EPOCH_YEAR;
> - while (days < 0 || days >= year_lengths[yleap = 
> IPS_IS_LEAP_YEAR(year)]) {
> - int newy;
> -
> - newy = year + (days / IPS_DAYS_NORMAL_YEAR);
> - if (days < 0)
> - --newy;
> - days -= (newy - year) * IPS_DAYS_NORMAL_YEAR +
> - IPS_NUM_LEAP_YEARS_THROUGH(newy - 1) -
> - IPS_NUM_LEAP_

Re: [PATCH] tcmu: Fix trailing semicolon

2018-01-18 Thread Nicholas A. Bellinger
On Tue, 2018-01-16 at 10:25 -0600, Michael Christie wrote:
> On 01/16/2018 09:34 AM, Luis de Bethencourt wrote:
> > The trailing semicolon is an empty statement that does no operation.
> > It is completely stripped out by the compiler. Removing it since it doesn't 
> > do
> > anything.
> > 
> > Signed-off-by: Luis de Bethencourt 
> > ---
> > 
> > Hi,
> > 
> > After fixing the same thing in drivers/staging/rtl8723bs/, Joe Perches
> > suggested I fix it treewide [0].
> > 
> > Best regards 
> > Luis
> > 
> > 
> > [0] 
> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115410.html
> > [1] 
> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115390.html
> > 
> >  drivers/target/target_core_user.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 

Applied.  Thanks Luis + MNC.