Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1343331 > unrolled thread

[REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

Started byLaura Abbott <labbott@redhat.com>
First post2016-02-25 18:20 +0100
Last post2016-02-27 14:10 +0100
Articles 8 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them  as removable" changes userspace behavior Laura Abbott <labbott@redhat.com> - 2016-02-25 18:20 +0100
    Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag  them as removable" changes userspace behavior Manuel Lauss <manuel.lauss@gmail.com> - 2016-02-25 19:50 +0100
    Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag  them as removable" changes userspace behavior Manuel Lauss <manuel.lauss@gmail.com> - 2016-02-25 20:00 +0100
      Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag  them as removable" changes userspace behavior Laura Abbott <labbott@redhat.com> - 2016-02-25 20:40 +0100
        Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag  them as removable" changes userspace behavior Tejun Heo <tj@kernel.org> - 2016-02-25 22:30 +0100
    Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag  them as removable" changes userspace behavior Manuel Lauss <manuel.lauss@gmail.com> - 2016-02-25 20:20 +0100
      Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag  them as removable" changes userspace behavior Laura Abbott <labbott@redhat.com> - 2016-02-26 02:50 +0100
        Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag  them as removable" changes userspace behavior Tejun Heo <tj@kernel.org> - 2016-02-27 14:10 +0100

#1343331 — [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromLaura Abbott <labbott@redhat.com>
Date2016-02-25 18:20 +0100
Subject[REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r67K2-71c-1@gated-at.bofh.it>
Hi,

We received a bugzilla report https://bugzilla.redhat.com/show_bug.cgi?id=1310682
of partitions being automounted unexpectedly. Testing showed that
8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was responsible.
This seems to be classified as 'breaking' userspace given that this behavior results
in partitions that were previously unmounted now being being mounted unwanted
automatically which is unwanted behavior for the user. Can we revert this patch or
give a fix to change this behavior?

Thanks,
Laura

[toc] | [next] | [standalone]


#1343389 — Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromManuel Lauss <manuel.lauss@gmail.com>
Date2016-02-25 19:50 +0100
SubjectRe: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r6997-80u-7@gated-at.bofh.it>
In reply to#1343331
On Thu, Feb 25, 2016 at 6:12 PM, Laura Abbott <labbott@redhat.com> wrote:
> Hi,
>
> We received a bugzilla report
> https://bugzilla.redhat.com/show_bug.cgi?id=1310682
> of partitions being automounted unexpectedly. Testing showed that
> 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was
> responsible.
> This seems to be classified as 'breaking' userspace given that this behavior
> results
> in partitions that were previously unmounted now being being mounted
> unwanted
> automatically which is unwanted behavior for the user. Can we revert this
> patch or
> give a fix to change this behavior?

This patch doesn't cause esata disks to be automounted on my distro (gentoo),
it just let's them appear in KDE's removable device widget for me
to mount it at a klick's notice.
I'm not convinced this is a "breaking userspace" problem.

Manuel

[toc] | [prev] | [next] | [standalone]


#1343399 — Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromManuel Lauss <manuel.lauss@gmail.com>
Date2016-02-25 20:00 +0100
SubjectRe: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r69iO-84U-27@gated-at.bofh.it>
In reply to#1343331
On Thu, Feb 25, 2016 at 6:12 PM, Laura Abbott <labbott@redhat.com> wrote:
> Hi,
>
> We received a bugzilla report
> https://bugzilla.redhat.com/show_bug.cgi?id=1310682
> of partitions being automounted unexpectedly. Testing showed that
> 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was
> responsible.
> This seems to be classified as 'breaking' userspace given that this behavior
> results
> in partitions that were previously unmounted now being being mounted
> unwanted
> automatically which is unwanted behavior for the user. Can we revert this
> patch or
> give a fix to change this behavior?

Do the affected users have sata ports set to hotplug capable? Because that's
what the patch does: it flags ports which have the hotplug capability enabled
as "removable", and I guess redhat just automounts anything that is flagged
as a removable disk (like an usb drive).

Manuel

[toc] | [prev] | [next] | [standalone]


#1343429 — Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromLaura Abbott <labbott@redhat.com>
Date2016-02-25 20:40 +0100
SubjectRe: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r69Vv-9g-1@gated-at.bofh.it>
In reply to#1343399
On 02/25/2016 10:56 AM, Manuel Lauss wrote:
> On Thu, Feb 25, 2016 at 6:12 PM, Laura Abbott <labbott@redhat.com> wrote:
>> Hi,
>>
>> We received a bugzilla report
>> https://bugzilla.redhat.com/show_bug.cgi?id=1310682
>> of partitions being automounted unexpectedly. Testing showed that
>> 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was
>> responsible.
>> This seems to be classified as 'breaking' userspace given that this behavior
>> results
>> in partitions that were previously unmounted now being being mounted
>> unwanted
>> automatically which is unwanted behavior for the user. Can we revert this
>> patch or
>> give a fix to change this behavior?
>
> Do the affected users have sata ports set to hotplug capable? Because that's
> what the patch does: it flags ports which have the hotplug capability enabled
> as "removable", and I guess redhat just automounts anything that is flagged
> as a removable disk (like an usb drive).
>
> Manuel
>

The mounting comes from udisks. udisks will automount anything marked as removable
unless explicitly forbidden. You can argue whether or not this is good design but
it's still a change in behavior from what udisks was relying on to decide
whether or not to automount. lspci from the users hardware:

00:1f.2 SATA controller: Intel Corporation C600/X79 series chipset 6-Port SATA
AHCI Controller (rev 06)

I'll make a build with the patch you gave for testing.

Thanks,
Laura

[toc] | [prev] | [next] | [standalone]


#1343496 — Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromTejun Heo <tj@kernel.org>
Date2016-02-25 22:30 +0100
SubjectRe: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r6bDY-1mI-21@gated-at.bofh.it>
In reply to#1343429
Hello, Laura.

On Thu, Feb 25, 2016 at 11:37:09AM -0800, Laura Abbott wrote:
> The mounting comes from udisks. udisks will automount anything marked as removable
> unless explicitly forbidden. You can argue whether or not this is good design but
> it's still a change in behavior from what udisks was relying on to decide
> whether or not to automount. lspci from the users hardware:
> 
> 00:1f.2 SATA controller: Intel Corporation C600/X79 series chipset 6-Port SATA
> AHCI Controller (rev 06)

It is a behavior change but difficult to characterize as a breakage,
given that we do want userland to treat ports explicitly marked as
external to be treated as such.  That said, eSATA being mostly a
fringe thing, we can go "whatever" and revert it.

Let's hope it's a false positive that the patch can fix.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]


#1343415 — Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromManuel Lauss <manuel.lauss@gmail.com>
Date2016-02-25 20:20 +0100
SubjectRe: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r69Ca-8sB-9@gated-at.bofh.it>
In reply to#1343331
On Thu, Feb 25, 2016 at 6:12 PM, Laura Abbott <labbott@redhat.com> wrote:
> Hi,
>
> We received a bugzilla report
> https://bugzilla.redhat.com/show_bug.cgi?id=1310682
> of partitions being automounted unexpectedly. Testing showed that
> 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was
> responsible.
> This seems to be classified as 'breaking' userspace given that this behavior
> results
> in partitions that were previously unmounted now being being mounted
> unwanted
> automatically which is unwanted behavior for the user. Can we revert this
> patch or
> give a fix to change this behavior?

Can you test this?  I think the old HPCP bit should just be ignored,
although windows does use it, i.e. when I enable hotplug capability
in the bios on my desktop it shows the  "eject device" icon for a device
attached to the port on the motherboard, we probably can ignore it
on linux to not break all automounting distros.  On my laptop, where
I care about this, it still works as intended.

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 4029679..e029e3c 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1164,8 +1164,7 @@ static void ahci_port_init(struct device *dev,
struct ata_port *ap,

        /* mark esata ports */
        tmp = readl(port_mmio + PORT_CMD);
-       if ((tmp & PORT_CMD_HPCP) ||
-           ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)))
+       if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
                ap->pflags |= ATA_PFLAG_EXTERNAL;
 }

[toc] | [prev] | [next] | [standalone]


#1343720 — Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromLaura Abbott <labbott@redhat.com>
Date2016-02-26 02:50 +0100
SubjectRe: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r6fHz-4fM-1@gated-at.bofh.it>
In reply to#1343415
On 02/25/2016 11:09 AM, Manuel Lauss wrote:
> On Thu, Feb 25, 2016 at 6:12 PM, Laura Abbott <labbott@redhat.com> wrote:
>> Hi,
>>
>> We received a bugzilla report
>> https://bugzilla.redhat.com/show_bug.cgi?id=1310682
>> of partitions being automounted unexpectedly. Testing showed that
>> 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was
>> responsible.
>> This seems to be classified as 'breaking' userspace given that this behavior
>> results
>> in partitions that were previously unmounted now being being mounted
>> unwanted
>> automatically which is unwanted behavior for the user. Can we revert this
>> patch or
>> give a fix to change this behavior?
>
> Can you test this?  I think the old HPCP bit should just be ignored,
> although windows does use it, i.e. when I enable hotplug capability
> in the bios on my desktop it shows the  "eject device" icon for a device
> attached to the port on the motherboard, we probably can ignore it
> on linux to not break all automounting distros.  On my laptop, where
> I care about this, it still works as intended.
>
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 4029679..e029e3c 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -1164,8 +1164,7 @@ static void ahci_port_init(struct device *dev,
> struct ata_port *ap,
>
>          /* mark esata ports */
>          tmp = readl(port_mmio + PORT_CMD);
> -       if ((tmp & PORT_CMD_HPCP) ||
> -           ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)))
> +       if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
>                  ap->pflags |= ATA_PFLAG_EXTERNAL;
>   }
>

At least one user reported that this fixed the issue and he said
you could add

Tested-by: Bob Schultz <bob@engineer.com>

Thanks,
Laura

[toc] | [prev] | [next] | [standalone]


#1345016 — Re: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

FromTejun Heo <tj@kernel.org>
Date2016-02-27 14:10 +0100
SubjectRe: [REGRESSION] 8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior
Message-ID<r6MNb-3rS-13@gated-at.bofh.it>
In reply to#1343720
Hello,

On Thu, Feb 25, 2016 at 05:49:34PM -0800, Laura Abbott wrote:
> At least one user reported that this fixed the issue and he said
> you could add
> 
> Tested-by: Bob Schultz <bob@engineer.com>

Great, Manuel, can you please send a proper patch?

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web