[PATCH] blk-mq: fix a hung issue when set device state to blocked and restore running
When I use dd test a SCSI device which use blk-mq in the following steps:
1.echo "blocked" >/sys/block/sda/device/state
2.dd if=/dev/sda of=/mnt/t.log bs=1M count=10
3.echo "running" >/sys/block/sda/device/state
dd should finish this work after step 3, unfortunately, still hung.
After step2, the key code process is like this:
blk_mq_dispatch_rq_list-->scsi_queue_rq-->prep_to_mq
-->if ret is BLK_STS_RESOURCE, delay run hw queue
prep_to_mq will return BLK_STS_RESOURCE, and scsi_queue_rq will transter
it to BLK_STS_DEV_RESOURCE. In this situtation, we should delay run hw
queue. This patch fixes that.
Fixes: 86ff7c2a80cd ("blk-mq: introduce BLK_STS_DEV_RESOURCE")
Signed-off-by: zhengbin
---
block/blk-mq.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index a9c1816..556d606 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1309,15 +1309,17 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q,
struct list_head *list,
* returning BLK_STS_RESOURCE. Two exceptions are scsi-mq
* and dm-rq.
*
-* If driver returns BLK_STS_RESOURCE and SCHED_RESTART
-* bit is set, run queue after a delay to avoid IO stalls
-* that could otherwise occur if the queue is idle.
+* If driver returns BLK_STS_RESOURCE or BLK_STS_DEV_RESOURCE
+* and SCHED_RESTART bit is set, run queue after a delay to
+* avoid IO stalls that could otherwise occur if the queue
+* is idle.
*/
needs_restart = blk_mq_sched_needs_restart(hctx);
if (!needs_restart ||
(no_tag && list_empty_careful(&hctx->dispatch_wait.entry)))
blk_mq_run_hw_queue(hctx, true);
- else if (needs_restart && (ret == BLK_STS_RESOURCE))
+ else if (needs_restart && ((ret == BLK_STS_RESOURCE) ||
+ (ret == BLK_STS_DEV_RESOURCE)))
blk_mq_delay_run_hw_queue(hctx, BLK_MQ_RESOURCE_DELAY);
blk_mq_update_dispatch_busy(hctx, true);
--
2.7.4
[PATCH] squashfs: fix mtime underflow on 64 bit system
If we change the file mtime to 1969, mksquashfs and mount, the atime/mtime of this file will be underflow. The reason is treating timestamps with the high bit set as positive times(before 1970), which should be set as negative times just like on 32 bit system. After this, the poissble range of timestamps will be 1901-2038(prev is 1970-2106) on 64 bit system. Signed-off-by: zhengbin --- fs/squashfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/squashfs/inode.c b/fs/squashfs/inode.c index e9793b1e49a5..03a6ef77a2d2 100644 --- a/fs/squashfs/inode.c +++ b/fs/squashfs/inode.c @@ -72,7 +72,7 @@ static int squashfs_new_inode(struct super_block *sb, struct inode *inode, i_uid_write(inode, i_uid); i_gid_write(inode, i_gid); inode->i_ino = le32_to_cpu(sqsh_ino->inode_number); - inode->i_mtime.tv_sec = le32_to_cpu(sqsh_ino->mtime); + inode->i_mtime.tv_sec = (signed int)le32_to_cpu(sqsh_ino->mtime); inode->i_atime.tv_sec = inode->i_mtime.tv_sec; inode->i_ctime.tv_sec = inode->i_mtime.tv_sec; inode->i_mode = le16_to_cpu(sqsh_ino->mode); -- 2.16.2.dirty
[PATCH] lockdep: remove unnecessary unlikely()
DEBUG_LOCKS_WARN_ON() already contains an unlikely(), there is no need for another one. Signed-off-by: zhengbin --- kernel/locking/lockdep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index e221be7..1522034 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3157,7 +3157,7 @@ void lockdep_hardirqs_on(unsigned long ip) /* * See the fine text that goes along with this variable definition. */ - if (DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))) + if (DEBUG_LOCKS_WARN_ON(early_boot_irqs_disabled)) return; /* -- 2.7.4
[PATCH] time: compat settimeofday: Validate the values of tv from user
Similar to commit 6ada1fc0e1c4
("time: settimeofday: Validate the values of tv from user"),
an unvalidated user input is multiplied by a constant, which can result
in an undefined behaviour for large values. While this is validated
later, we should avoid triggering undefined behaviour.
Signed-off-by: zhengbin
---
kernel/time/time.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 7f7d691..5c54ca6 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -251,6 +251,10 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct old_timeval32
__user *, tv,
if (tv) {
if (compat_get_timeval(&user_tv, tv))
return -EFAULT;
+
+ if (!timeval_valid(&user_tv))
+ return -EINVAL;
+
new_ts.tv_sec = user_tv.tv_sec;
new_ts.tv_nsec = user_tv.tv_usec * NSEC_PER_USEC;
}
--
2.7.4
[PATCH v2] time: compat settimeofday: Validate the values of tv from user
Similar to commit 6ada1fc0e1c4
("time: settimeofday: Validate the values of tv from user"),
for a wide range of negative tv_usec values the multiplication overflow
turns them in positive numbers. So the 'validated later' is not catching
the invalid input.
Signed-off-by: zhengbin
---
kernel/time/time.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 7f7d691..5c54ca6 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -251,6 +251,10 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct old_timeval32
__user *, tv,
if (tv) {
if (compat_get_timeval(&user_tv, tv))
return -EFAULT;
+
+ if (!timeval_valid(&user_tv))
+ return -EINVAL;
+
new_ts.tv_sec = user_tv.tv_sec;
new_ts.tv_nsec = user_tv.tv_usec * NSEC_PER_USEC;
}
--
2.7.4
[PATCH] auxdisplay: panel: need to delete scan_timer when misc_register fails in panel_attach
In panel_attach, if misc_register fails, we need to delete scan_timer, which was setup in keypad_init->init_scan_timer. Reported-by: Hulk Robot Signed-off-by: zhengbin --- drivers/auxdisplay/panel.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c index e06de63..e6bd727 100644 --- a/drivers/auxdisplay/panel.c +++ b/drivers/auxdisplay/panel.c @@ -1617,6 +1617,8 @@ static void panel_attach(struct parport *port) return; err_lcd_unreg: + if (scan_timer.function) + del_timer_sync(&scan_timer); if (lcd.enabled) charlcd_unregister(lcd.charlcd); err_unreg_device: -- 2.7.4
Re: [PATCH] squashfs: fix mtime underflow on 64 bit system
Ping? On 2019/1/17 16:21, zhengbin wrote: > If we change the file mtime to 1969, mksquashfs and mount, > the atime/mtime of this file will be underflow. The reason is > treating timestamps with the high bit set as positive > times(before 1970), which should be set as negative times > just like on 32 bit system. After this, the poissble range > of timestamps will be 1901-2038(prev is 1970-2106) on 64 > bit system. > > Signed-off-by: zhengbin > --- > fs/squashfs/inode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/squashfs/inode.c b/fs/squashfs/inode.c > index e9793b1e49a5..03a6ef77a2d2 100644 > --- a/fs/squashfs/inode.c > +++ b/fs/squashfs/inode.c > @@ -72,7 +72,7 @@ static int squashfs_new_inode(struct super_block *sb, > struct inode *inode, > i_uid_write(inode, i_uid); > i_gid_write(inode, i_gid); > inode->i_ino = le32_to_cpu(sqsh_ino->inode_number); > - inode->i_mtime.tv_sec = le32_to_cpu(sqsh_ino->mtime); > + inode->i_mtime.tv_sec = (signed int)le32_to_cpu(sqsh_ino->mtime); > inode->i_atime.tv_sec = inode->i_mtime.tv_sec; > inode->i_ctime.tv_sec = inode->i_mtime.tv_sec; > inode->i_mode = le16_to_cpu(sqsh_ino->mode); > -- > 2.16.2.dirty > > > . >
Re: [PATCH] squashfs: fix mtime underflow on 64 bit system
hi Phillip, when you have time, help to confirm it? Looking forward to your reply. On 2019/1/17 16:21, zhengbin wrote: > If we change the file mtime to 1969, mksquashfs and mount, > the atime/mtime of this file will be underflow. The reason is > treating timestamps with the high bit set as positive > times(before 1970), which should be set as negative times > just like on 32 bit system. After this, the poissble range > of timestamps will be 1901-2038(prev is 1970-2106) on 64 > bit system. > > Signed-off-by: zhengbin > --- > fs/squashfs/inode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/squashfs/inode.c b/fs/squashfs/inode.c > index e9793b1e49a5..03a6ef77a2d2 100644 > --- a/fs/squashfs/inode.c > +++ b/fs/squashfs/inode.c > @@ -72,7 +72,7 @@ static int squashfs_new_inode(struct super_block *sb, > struct inode *inode, > i_uid_write(inode, i_uid); > i_gid_write(inode, i_gid); > inode->i_ino = le32_to_cpu(sqsh_ino->inode_number); > - inode->i_mtime.tv_sec = le32_to_cpu(sqsh_ino->mtime); > + inode->i_mtime.tv_sec = (signed int)le32_to_cpu(sqsh_ino->mtime); > inode->i_atime.tv_sec = inode->i_mtime.tv_sec; > inode->i_ctime.tv_sec = inode->i_mtime.tv_sec; > inode->i_mode = le16_to_cpu(sqsh_ino->mode); > -- > 2.16.2.dirty > > > . >
Re: [PATCH] time: compat settimeofday: Validate the values of tv from user
On 2019/7/5 20:14, Thomas Gleixner wrote:
> Zhengbin,
>
> On Fri, 5 Jul 2019, zhengbin wrote:
>
>> Similar to commit 6ada1fc0e1c4
>> ("time: settimeofday: Validate the values of tv from user"),
>> an unvalidated user input is multiplied by a constant, which can result
>> in an undefined behaviour for large values. While this is validated
>> later, we should avoid triggering undefined behaviour.
> I surely agree with the patch, but the argument that this is validated
> later and we just should avoid UB in general is just wrong.
>
> For a wide range of negative tv_usec values the multiplication overflow
> turns them in positive numbers. So the 'validated later' is not catching
> the invalid input.
>
> So 'should avoid ' is just the wrong argument here.
>
> Validation _is_ required before the multiplication so UB won't turn an
> invalid value into a valid one.
>
> Thanks,
>
> tglx
Strongly agree with this, I send a v2 patch, modify the comment?
>
> .
>
Re: [PATCH 3/8] aio_poll(): sanitize the logics after vfs_poll(), get rid of leak on error
+ if (async && !apt.error) --->may be this should be if (!async &&
!apt.error) ?
On 2019/3/7 8:03, Al Viro wrote:
> From: Al Viro
>
> We want iocb_put() happening on errors, to balance the extra reference
> we'd taken. As it is, we end up with a leak. The rules should be
> * error: iocb_put() to deal with the extra ref, return error,
> let the caller do another iocb_put().
> * async: iocb_put() to deal with the extra ref, return 0.
> * no error, event present immediately: aio_poll_complete() to
> report it, iocb_put() to deal with the extra ref, return 0.
>
> Signed-off-by: Al Viro
> ---
> fs/aio.c | 25 +++--
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 3a8b894378e0..22b288997441 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1724,6 +1724,7 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb, const
> struct iocb *iocb)
> struct kioctx *ctx = aiocb->ki_ctx;
> struct poll_iocb *req = &aiocb->poll;
> struct aio_poll_table apt;
> + bool async = false;
> __poll_t mask;
>
> /* reject any unknown events outside the normal event mask. */
> @@ -1760,30 +1761,26 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb,
> const struct iocb *iocb)
>
> spin_lock_irq(&ctx->ctx_lock);
> spin_lock(&req->head->lock);
> - if (req->woken) {
> - /* wake_up context handles the rest */
> - mask = 0;
> + if (req->woken) { /* already taken up by aio_poll_wake() */
> + async = true;
> apt.error = 0;
> - } else if (mask || apt.error) {
> - /* if we get an error or a mask we are done */
> - WARN_ON_ONCE(list_empty(&req->wait.entry));
> - list_del_init(&req->wait.entry);
> - } else {
> - /* actually waiting for an event */
> + } else if (!mask && !apt.error) { /* actually waiting for an event */
> list_add_tail(&aiocb->ki_list, &ctx->active_reqs);
> aiocb->ki_cancel = aio_poll_cancel;
> + async = true;
> + } else { /* if we get an error or a mask we are done */
> + WARN_ON_ONCE(list_empty(&req->wait.entry));
> + list_del_init(&req->wait.entry);
> + /* no wakeup in the future either; aiocb is ours to dispose of
> */
> }
> spin_unlock(&req->head->lock);
> spin_unlock_irq(&ctx->ctx_lock);
>
> out:
> - if (unlikely(apt.error))
> - return apt.error;
> -
> - if (mask)
> + if (async && !apt.error)
> aio_poll_complete(aiocb, mask);
> iocb_put(aiocb);
> - return 0;
> + return apt.error;
> }
>
> static int __io_submit_one(struct kioctx *ctx, const struct iocb *iocb,
>
Re: [PATCH 2/8] aio_poll_wake(): don't set ->woken if we ignore the wakeup
So, what should we do? On 2019/3/7 10:18, Al Viro wrote: > On Thu, Mar 07, 2019 at 12:03:10AM +, Al Viro wrote: >> From: Al Viro >> >> In case of early wakeups, aio_poll() assumes that aio_poll_complete() >> has either already happened or is imminent. In that case we do not >> want to put iocb on the list of cancellables. However, ignored >> wakeups need to be treated as if wakeup has not happened at all. >> Trivially fixed by having aio_poll_wake() set ->woken only after >> it's committed to taking iocb out of the waitqueue. >> >> Spotted-by: zhengbin >> Signed-off-by: Al Viro > > ... and unfortunately it's worse than just that - what both of us > have missed is that one could have non-specific wakep + schedule_work + > aio_poll_complete_work() rechecking ->poll(), seeing nothing of > interest and reinserting into queue. All before vfs_poll() manages > to return into aio_poll(). The window is harder to hit, but it's > still there, with exact same "failed to add to cancel list" kind of bug > if we do hit it ;-/ > > . >
Re: [PATCH RESEND] 9p: Fix memory leak in v9fs_mount
Is this OK? I don't see it on linux-next On 2020/6/15 18:20, Dominique Martinet wrote: Zheng Bin wrote on Mon, Jun 15, 2020: v9fs_mount v9fs_session_init v9fs_cache_session_get_cookie v9fs_random_cachetag -->alloc cachetag v9ses->fscache = fscache_acquire_cookie -->maybe NULL sb = sget-->fail, goto clunk clunk_fid: v9fs_session_close if (v9ses->fscache)-->NULL kfree(v9ses->cachetag) Thus memleak happens. Signed-off-by: Zheng Bin Thanks, will run tests & queue next weekend
Re: [PATCH v2] nbd: Fix memory leak in nbd_add_socket
On 2020/6/20 20:05, Markus Elfring wrote:
If we add first socket to nbd, config->socks is malloced but
num_connections does not update(nsock's allocation fail), the memory
is leaked. Cause in later nbd_config_put(), will only free config->socks
when num_connections is not 0.
Let nsock's allocation first to avoid this.
I suggest to improve this change description.
Can an other wording variant be nicer?
em, how about this?
When adding first socket to nbd, if nsock's allocation fails, config->socks
is malloced but num_connections does not update, memory leak will
occur(Function
nbd_config_put will only free config->socks when num_connections is not 0).
…
+++ b/drivers/block/nbd.c
@@ -1037,21 +1037,22 @@ static int nbd_add_socket(struct nbd_device *nbd,
unsigned long arg,
return -EBUSY;
}
+ nsock = kzalloc(sizeof(struct nbd_sock), GFP_KERNEL);
Please use the following code variant.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=4333a9b0b67bb4e8bcd91bdd80da80b0ec151162#n854
+ nsock = kzalloc(sizeof(*nsock), GFP_KERNEL);
…
if (!socks) {
sockfd_put(sock);
+ kfree(nsock);
return -ENOMEM;
}
Please take another software design possibility into account.
if (!socks) {
- sockfd_put(sock);
- return -ENOMEM;
+ kfree(nsock);
+ goto put_socket;
}
Regards,
Markus
.
[tip:locking/core] locking/lockdep: Remove unnecessary unlikely()
Commit-ID: d671002be6bdd7f77a771e23bf3e95d1f16775e6 Gitweb: https://git.kernel.org/tip/d671002be6bdd7f77a771e23bf3e95d1f16775e6 Author: zhengbin AuthorDate: Mon, 29 Apr 2019 20:26:31 +0800 Committer: Ingo Molnar CommitDate: Mon, 29 Apr 2019 16:11:01 +0200 locking/lockdep: Remove unnecessary unlikely() DEBUG_LOCKS_WARN_ON() already contains an unlikely(), there is no need for another one. Signed-off-by: zhengbin Cc: Andrew Morton Cc: Linus Torvalds Cc: Paul E. McKenney Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Will Deacon Cc: hout...@huawei.com Link: http://lkml.kernel.org/r/1556540791-23110-1-git-send-email-zhengbi...@huawei.com Signed-off-by: Ingo Molnar --- kernel/locking/lockdep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 25ecc6d3058b..6426d071a324 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3256,7 +3256,7 @@ void lockdep_hardirqs_on(unsigned long ip) /* * See the fine text that goes along with this variable definition. */ - if (DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))) + if (DEBUG_LOCKS_WARN_ON(early_boot_irqs_disabled)) return; /*
[tip:timers/core] time: Validate user input in compat_settimeofday()
Commit-ID: 9176ab1b848059a0cd9caf39f0cebaa1b7ec5ec2
Gitweb: https://git.kernel.org/tip/9176ab1b848059a0cd9caf39f0cebaa1b7ec5ec2
Author: zhengbin
AuthorDate: Sun, 7 Jul 2019 08:51:41 +0800
Committer: Thomas Gleixner
CommitDate: Sun, 7 Jul 2019 12:05:40 +0200
time: Validate user input in compat_settimeofday()
The user value is validated after converting the timeval to a timespec, but
for a wide range of negative tv_usec values the multiplication overflow turns
them in positive numbers. So the 'validated later' is not catching the
invalid input.
Signed-off-by: zhengbin
Signed-off-by: Thomas Gleixner
Link:
https://lkml.kernel.org/r/1562460701-113301-1-git-send-email-zhengbi...@huawei.com
---
kernel/time/time.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 7f7d6914ddd5..5c54ca632d08 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -251,6 +251,10 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct old_timeval32
__user *, tv,
if (tv) {
if (compat_get_timeval(&user_tv, tv))
return -EFAULT;
+
+ if (!timeval_valid(&user_tv))
+ return -EINVAL;
+
new_ts.tv_sec = user_tv.tv_sec;
new_ts.tv_nsec = user_tv.tv_usec * NSEC_PER_USEC;
}
