On Fri, 2018-01-12 at 08:23 +0200, Leon Romanovsky wrote:
> On Thu, Jan 11, 2018 at 07:07:06PM +0000, Bart Van Assche wrote:
> > On Thu, 2018-01-11 at 21:00 +0200, Leon Romanovsky wrote:
> > > On Thu, Jan 11, 2018 at 04:02:33PM +0000, Bart Van Assche wrote:
> > > > On Thu, 2018-01-11 at 08:22 +0200, Leon Romanovsky wrote:
> > > > > The proposed patch definitely decreases the chance of races, but it is not fixing them.
> > > > > There is a chance to have change in qp state immediately after your "if ..." check.
> > > >
> > > > Hello Leon,
> > > >
> > > > Please have a look at rxe_qp_error() and you will see that the patch I posted
> > > > is a proper fix. In the scenario you described rxe_qp_error() will trigger a
> > > > run of rxe_completer().
> > >
> > > Bart,
> > >
> > > What am I missing?
> > >
> > > CPU1 CPU2
> > > if (unlikely....
> > > <---
> > > /* move the qp to the error state */
> > > void rxe_qp_error(struct rxe_qp *qp)
> > > {
> > > qp->req.state = QP_STATE_ERROR;
> > > qp->resp.state = QP_STATE_ERROR;
> > > qp->attr.qp_state = IB_QPS_ERR;
> > > --->
> > > rxe_run_task(&qp->req.task, must_sched);
> > >
> > >
> > >
> > > It is more or less the same as without "if (unlikely..."
> >
> > Hello Leon,
> >
> > In the above the part of rxe_qp_error() that I was referring to in my e-mail
> > is missing:
> >
> > if (qp_type(qp) == IB_QPT_RC)
> > rxe_run_task(&qp->comp.task, 1);
>
>
> But it is exactly where race exists, as long QP isn't protected, it can
> switch CPUs and create race.
Hello Leon,
Can you clarify which race you are referring to? rxe_run_task() uses the
tasklet mechanism and tasklets are guaranteed to run on at most one CPU at a
time. See also the "Top and Bottom Halves" chapter in Linux Device Drivers,
3rd edition. See also the tasklet_schedule() implementation in
<linux/interrupt.h> and in kernel/softirq.c.
Thanks,
Bart.
Hi Arnd,
On Thursday 11 January 2018 11:46 PM, Eric Anholt wrote:
> Arnd Bergmann <arnd(a)arndb.de> writes:
>
>> On Thu, Jan 11, 2018 at 2:30 PM, Kishon Vijay Abraham I <kishon(a)ti.com> wrote:
>>> On Thursday 11 January 2018 02:27 AM, Arnd Bergmann wrote:
>>>> On Mon, Jan 8, 2018 at 7:32 PM, Kishon Vijay Abraham I <kishon(a)ti.com> wrote:
>>>>> On Monday 08 January 2018 06:31 PM, Arnd Bergmann wrote:
>>>>>> Stefan Wahren reports a problem with a warning fix that was merged
>>>>>> ---
>>>>>> This obviously needs to be tested, I wrote this up as a reply to
>>>>>> Stefan's bug report. I'm fairly sure that I covered all usb-phy
>>>>>> driver strings here. My goal is to have a fix merged into 4.15
>>>>>> rather than reverting all the DT fixes.
>>>>>
>>>>> Shouldn't the fix be in phy consumer drivers to not return error if it's able
>>>>> to find the phy either using usb-phy or generic phy?
>>>>
>>>> Stefan has posted a patch to that effect now, but I fear that might be
>>>> a little fragile, in particular this short before the release with the
>>>> regression
>>>> in place.
>>>>
>>>> The main problem is that we'd have to change the generic
>>>> usb_add_hcd() function in addition to dwc2 and dwc3 to ignore
>>>> -EPROBE_DEFER from phy_get() whenever usb_get_phy_dev()
>>>> has already succeeded.
>>>>
>>>> If there is any HCD that relies on usb_add_hcd() to get both the
>>>> usb_phy and the phy structures, and it may need to defer probing
>>>> when the latter one isn't ready yet, that fix would break another
>>>> driver.
>>>
>>> hmm.. IMO the better thing right now would be to revert the dt patch which adds
>>> #phy-cells.
>>> We have to see if there are better fixes in order to add #phy-cells warning fix
>>> in stable tree.
>>
>> Let's see which patches that would be, I think this is the full list of
>> nodes that got an extra #phy-cells:
>>
>> c22fe696157d ARM: dts: Fix dm814x missing phy-cells property
>> f0e11ff8ff65 ARM: dts: am33xx: Add missing #phy-cells to ti,am335x-usb-phy
>> c5bbf358b790 arm: dts: nspire: Add missing #phy-cells to usb-nop-xceiv
>> 44e5dced2ef6 arm: dts: marvell: Add missing #phy-cells to usb-nop-xceiv
>> 014d6da6cb25 ARM: dts: bcm283x: Fix DTC warnings about missing phy-cells
>> f568f6f554b8 ARM: dts: omap: Add missing #phy-cells to usb-nop-xceiv
>>
>> plus a couple in linux-next:
>>
>> d745d5f277bf ARM: dts: imx51-zii-rdu1: Add missing #phy-cells to usb-nop-xceiv
>> 915fbe59cbf2 ARM: dts: imx: Add missing #phy-cells to usb-nop-xceiv
>>
>> It's a lot of patches to revert, and I guess it would get us back to hundreds
>> of warnings in an allmodconfig build, so I'd first try to come up with
>> ways to prove that at least some of them can stay.
>>
>> Almost all the warnings are about "usb-nop-xceiv" phys, the only exceptions
>> I could find are the OMAP ones (the first two patches), which use
>> "ti,am335x-usb-phy" and are referenced from a "ti,musb-am33xx". That
>> particular driver is not affected by the bug, so we can leave that in.
>>
>> To deal with all the "usb-nop-xceiv" references including the one that
>> Stefan reported, we could use a much simpler version of my earlier
>> patch, do you think this is any better?
yeah, this looks simpler.
>>
>> Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>
In case you want to take this patch yourself
Acked-by: Kishon Vijay Abraham I <kishon(a)ti.com>
(or let me know if I have to create a separate pull request for Greg)
Thanks
Kishon
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index b4964b067aec..f056d8fb3921 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -410,6 +410,10 @@ static struct phy *_of_phy_get(struct device_node
>> *np, int index)
>> if (ret)
>> return ERR_PTR(-ENODEV);
>>
>> + /* This phy type handled by the usb-phy subsystem for now */
>> + if (of_device_is_compatible("usb-nop-xceiv"))
>> + return ERR_PTR(-ENODEV);
>> +
>> mutex_lock(&phy_provider_mutex);
>> phy_provider = of_phy_provider_lookup(args.np);
>> if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
>
> This seems like a nice workaround!
>
The patch titled
Subject: pipe: fix off-by-one error when checking buffer limits
has been added to the -mm tree. Its filename is
pipe-fix-off-by-one-error-when-checking-buffer-limits.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/pipe-fix-off-by-one-error-when-che…
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/pipe-fix-off-by-one-error-when-che…
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/SubmitChecklist when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Eric Biggers <ebiggers(a)google.com>
Subject: pipe: fix off-by-one error when checking buffer limits
With pipe-user-pages-hard set to 'N', users were actually only allowed up
to 'N - 1' buffers; and likewise for pipe-user-pages-soft.
Fix this to allow up to 'N' buffers, as would be expected.
Link: http://lkml.kernel.org/r/20180111052902.14409-5-ebiggers3@gmail.com
Fixes: b0b91d18e2e9 ("pipe: fix limit checking in pipe_set_size()")
Signed-off-by: Eric Biggers <ebiggers(a)google.com>
Acked-by: Willy Tarreau <w(a)1wt.eu>
Acked-by: Kees Cook <keescook(a)chromium.org>
Acked-by: Joe Lawrence <joe.lawrence(a)redhat.com>
Cc: Alexander Viro <viro(a)zeniv.linux.org.uk>
Cc: "Luis R . Rodriguez" <mcgrof(a)kernel.org>
Cc: Michael Kerrisk <mtk.manpages(a)gmail.com>
Cc: Mikulas Patocka <mpatocka(a)redhat.com>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
fs/pipe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -puN fs/pipe.c~pipe-fix-off-by-one-error-when-checking-buffer-limits fs/pipe.c
--- a/fs/pipe.c~pipe-fix-off-by-one-error-when-checking-buffer-limits
+++ a/fs/pipe.c
@@ -605,12 +605,12 @@ static unsigned long account_pipe_buffer
static bool too_many_pipe_buffers_soft(unsigned long user_bufs)
{
- return pipe_user_pages_soft && user_bufs >= pipe_user_pages_soft;
+ return pipe_user_pages_soft && user_bufs > pipe_user_pages_soft;
}
static bool too_many_pipe_buffers_hard(unsigned long user_bufs)
{
- return pipe_user_pages_hard && user_bufs >= pipe_user_pages_hard;
+ return pipe_user_pages_hard && user_bufs > pipe_user_pages_hard;
}
static bool is_unprivileged_user(void)
_
Patches currently in -mm which might be from ebiggers(a)google.com are
userfaultfd-convert-to-use-anon_inode_getfd.patch
pipe-sysctl-drop-min-parameter-from-pipe-max-size-converter.patch
pipe-sysctl-remove-pipe_proc_fn.patch
pipe-actually-allow-root-to-exceed-the-pipe-buffer-limits.patch
pipe-fix-off-by-one-error-when-checking-buffer-limits.patch
pipe-reject-f_setpipe_sz-with-size-over-uint_max.patch
pipe-simplify-round_pipe_size.patch
pipe-read-buffer-limits-atomically.patch
The patch titled
Subject: pipe: actually allow root to exceed the pipe buffer limits
has been added to the -mm tree. Its filename is
pipe-actually-allow-root-to-exceed-the-pipe-buffer-limits.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/pipe-actually-allow-root-to-exceed…
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/pipe-actually-allow-root-to-exceed…
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/SubmitChecklist when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Eric Biggers <ebiggers(a)google.com>
Subject: pipe: actually allow root to exceed the pipe buffer limits
pipe-user-pages-hard and pipe-user-pages-soft are only supposed to apply
to unprivileged users, as documented in both Documentation/sysctl/fs.txt
and the pipe(7) man page.
However, the capabilities are actually only checked when increasing a
pipe's size using F_SETPIPE_SZ, not when creating a new pipe. Therefore,
if pipe-user-pages-hard has been set, the root user can run into it and be
unable to create pipes. Similarly, if pipe-user-pages-soft has been set,
the root user can run into it and have their pipes limited to 1 page each.
Fix this by allowing the privileged override in both cases.
Link: http://lkml.kernel.org/r/20180111052902.14409-4-ebiggers3@gmail.com
Fixes: 759c01142a5d ("pipe: limit the per-user amount of pages allocated in pipes")
Signed-off-by: Eric Biggers <ebiggers(a)google.com>
Acked-by: Kees Cook <keescook(a)chromium.org>
Acked-by: Joe Lawrence <joe.lawrence(a)redhat.com>
Cc: Alexander Viro <viro(a)zeniv.linux.org.uk>
Cc: "Luis R . Rodriguez" <mcgrof(a)kernel.org>
Cc: Michael Kerrisk <mtk.manpages(a)gmail.com>
Cc: Mikulas Patocka <mpatocka(a)redhat.com>
Cc: Willy Tarreau <w(a)1wt.eu>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
fs/pipe.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff -puN fs/pipe.c~pipe-actually-allow-root-to-exceed-the-pipe-buffer-limits fs/pipe.c
--- a/fs/pipe.c~pipe-actually-allow-root-to-exceed-the-pipe-buffer-limits
+++ a/fs/pipe.c
@@ -613,6 +613,11 @@ static bool too_many_pipe_buffers_hard(u
return pipe_user_pages_hard && user_bufs >= pipe_user_pages_hard;
}
+static bool is_unprivileged_user(void)
+{
+ return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+}
+
struct pipe_inode_info *alloc_pipe_info(void)
{
struct pipe_inode_info *pipe;
@@ -629,12 +634,12 @@ struct pipe_inode_info *alloc_pipe_info(
user_bufs = account_pipe_buffers(user, 0, pipe_bufs);
- if (too_many_pipe_buffers_soft(user_bufs)) {
+ if (too_many_pipe_buffers_soft(user_bufs) && is_unprivileged_user()) {
user_bufs = account_pipe_buffers(user, pipe_bufs, 1);
pipe_bufs = 1;
}
- if (too_many_pipe_buffers_hard(user_bufs))
+ if (too_many_pipe_buffers_hard(user_bufs) && is_unprivileged_user())
goto out_revert_acct;
pipe->bufs = kcalloc(pipe_bufs, sizeof(struct pipe_buffer),
@@ -1065,7 +1070,7 @@ static long pipe_set_size(struct pipe_in
if (nr_pages > pipe->buffers &&
(too_many_pipe_buffers_hard(user_bufs) ||
too_many_pipe_buffers_soft(user_bufs)) &&
- !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
+ is_unprivileged_user()) {
ret = -EPERM;
goto out_revert_acct;
}
_
Patches currently in -mm which might be from ebiggers(a)google.com are
userfaultfd-convert-to-use-anon_inode_getfd.patch
pipe-sysctl-drop-min-parameter-from-pipe-max-size-converter.patch
pipe-sysctl-remove-pipe_proc_fn.patch
pipe-actually-allow-root-to-exceed-the-pipe-buffer-limits.patch
pipe-fix-off-by-one-error-when-checking-buffer-limits.patch
pipe-reject-f_setpipe_sz-with-size-over-uint_max.patch
pipe-simplify-round_pipe_size.patch
pipe-read-buffer-limits-atomically.patch
Starting from commit 041e4575f034 ("mtd: nand: handle ECC errors in
OOB"), nand_do_read_oob() (from the NAND core) did return 0 or a
negative error, and the MTD layer expected it.
However, the trend for the NAND layer is now to return an error or a
positive number of bitflips. Deciding which status to return to the user
belongs to the MTD layer.
Commit e47f68587b82 ("mtd: check for max_bitflips in mtd_read_oob()")
brought this logic to the mtd_read_oob() function while the status
coming from nand_do_read_oob() (called by the ->ecc.read_oob() hook) was
left unchanged.
Fixes: e47f68587b82 ("mtd: check for max_bitflips in mtd_read_oob()")
Cc: stable(a)vger.kernel.org
Signed-off-by: Miquel Raynal <miquel.raynal(a)free-electrons.com>
---
drivers/mtd/nand/nand_base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 469220065b8b..88256b656072 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3876,7 +3876,7 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
if (mtd->ecc_stats.failed - stats.failed)
return -EBADMSG;
- return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
+ return max_bitflips;
}
/**
--
2.11.0
Hi,
On 1/11/2018 12:16 AM, Felipe Balbi wrote:
>
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen(a)synopsys.com> writes:
>> In control read transfer completion handler, the driver needs to reset
>> the TRB enqueue counter. Since there is one control endpoint structure
>> for each direction, we must also track the TRB enqueue counter for each
>> direction. Currently the driver only resets the TRB counter for control
>> OUT endpoint only. Check for the data direction and properly reset the
>> TRB counter from correct control endpoint.
>>
>> Cc: stable(a)vger.kernel.org
>> Signed-off-by: Thinh Nguyen <thinhn(a)synopsys.com>
>
> missing Fixes tag
Same with this patch. There's no particular commit that it fixes.
BR,
Thinh
Hi,
On 1/11/2018 12:16 AM, Felipe Balbi wrote:
>
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen(a)synopsys.com> writes:
>> There are 2 control endpoint structures for DWC3. However, the driver
>> only updates the OUT direction control endpoint structure during
>> ConnectDone event. DWC3 driver needs to update the endpoint max packet
>> size for control IN endpoint as well. If the max packet size is not
>> properly set, then the driver will incorrectly calculate the data
>> transfer size and fail to send ZLP for HS/FS 3-stage control read
>> transfer.
>>
>> The fix is simply to update the max packet size for the ep0 IN direction
>> during ConnectDone event.
>>
>> Cc: stable(a)vger.kernel.org
>> Signed-off-by: Thinh Nguyen <thinhn(a)synopsys.com>
>
> missing Fixes tag
This bug has been there since the beginning. There's no particular
commit that it fixes.
BR,
Thinh
The patch below was submitted to be applied to the 4.14-stable tree.
I fail to see how this patch meets the stable kernel rules as found at
Documentation/process/stable-kernel-rules.rst.
I could be totally wrong, and if so, please respond to
<stable(a)vger.kernel.org> and let me know why this patch should be
applied. Otherwise, it is now dropped from my patch queues, never to be
seen again.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala(a)linux.intel.com>
Date: Thu, 16 Nov 2017 18:02:15 +0200
Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Moving the init_clock_gating() call from intel_modeset_init_hw() to
intel_modeset_gem_init() had an unintended effect of not applying
some workarounds on resume. This, for example, cause some kind of
corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
screen after hibernation. Fix the problem by explicitly calling
init_clock_gating() from the resume path.
I really hope this doesn't break something else again. At least
the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
didn't make a comeback, even after a hibernate cycle.
v2: Reorder the init_clock_gating vs. modeset_init_hw to match
the display reset path (Rodrigo)
Cc: stable(a)vger.kernel.org
Cc: Chris Wilson <chris(a)chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi(a)intel.com>
Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
Reviewed-by: Rodrigo Vivi <rodrigo.vivi(a)intel.com>
Reviewed-by: Chris Wilson <chris(a)chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171116160215.25715-1-ville.…
Signed-off-by: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
(cherry picked from commit 675f7ff35bd256e65d3d0f52718d8babf5d1002a)
Signed-off-by: Jani Nikula <jani.nikula(a)intel.com>
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 34191028bbad..7d9b07df32fa 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1714,6 +1714,7 @@ static int i915_drm_resume(struct drm_device *dev)
intel_guc_resume(dev_priv);
intel_modeset_init_hw(dev);
+ intel_init_clock_gating(dev_priv);
spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display.hpd_irq_setup)