As mentioned by Linus, swait is exclusive mode and had better behave like it
and be named like it.
Make it so.
---
arch/mips/kvm/mips.c | 4 ++--
arch/powerpc/kvm/book3s_hv.c | 6 +++---
arch/s390/kvm/interrupt.c | 2 +-
arch/x86/kernel/kvm.c | 4 ++--
arch/x86/kvm/lapic.c | 2 +-
include/linux/swait.h | 36 ++++++++++++++++++------------------
kernel/power/suspend.c | 4 ++--
kernel/rcu/srcutiny.c | 4 ++--
kernel/rcu/tree.c | 8 ++++----
kernel/rcu/tree_exp.h | 4 ++--
kernel/rcu/tree_plugin.h | 12 ++++++------
kernel/sched/swait.c | 32 ++++++++++++++++++++++----------
virt/kvm/arm/arm.c | 4 ++--
virt/kvm/arm/psci.c | 2 +-
virt/kvm/async_pf.c | 2 +-
virt/kvm/kvm_main.c | 4 ++--
16 files changed, 71 insertions(+), 59 deletions(-)
"
On Tue, Jun 12, 2018 at 1:39 AM Peter Zijlstra <[email protected]> wrote:
>
> As mentioned by Linus, swait is exclusive mode and had better behave like it
> and be named like it.
Ack on the patches.
I do note how quilt emails are really hard to read, because that:
Content-Disposition: inline
makes gmail think it's flowed.
Which works horribly badly for patches, surprise surprise.
So I really wish quilt wouldn't do that. It does smell like a gmail
bug, but at the same time, why would you use "Content-Disposition:
inline" when you don't have an actual multi-part email? So I do blame
quilt too for sending nonsensical headers.
(Yes, yes, I see the "It is permissible to use Content-Disposition on
the main body" in the RFC. But the RFC also makes it clear that it
actually matters for how things are presented, so saying "ok, I'll do
flowed" seems equally insane and equally technically RFC-compliant)
Linus
On Tue, Jun 12, 2018 at 09:47:34AM -0700, Linus Torvalds wrote:
> I do note how quilt emails are really hard to read, because that:
>
> Content-Disposition: inline
>
> makes gmail think it's flowed.
>
> Which works horribly badly for patches, surprise surprise.
>
> So I really wish quilt wouldn't do that. It does smell like a gmail
> bug, but at the same time, why would you use "Content-Disposition:
> inline" when you don't have an actual multi-part email? So I do blame
> quilt too for sending nonsensical headers.
>
> (Yes, yes, I see the "It is permissible to use Content-Disposition on
> the main body" in the RFC. But the RFC also makes it clear that it
> actually matters for how things are presented, so saying "ok, I'll do
> flowed" seems equally insane and equally technically RFC-compliant)
Quilt people, anything that can be done about that?
2018-06-12 18:47 GMT+02:00 Linus Torvalds <[email protected]>:
> "
> On Tue, Jun 12, 2018 at 1:39 AM Peter Zijlstra <[email protected]> wrote:
>>
>> As mentioned by Linus, swait is exclusive mode and had better behave like it
>> and be named like it.
>
> Ack on the patches.
>
> I do note how quilt emails are really hard to read, because that:
>
> Content-Disposition: inline
>
> makes gmail think it's flowed.
>
> Which works horribly badly for patches, surprise surprise.
>
> So I really wish quilt wouldn't do that. It does smell like a gmail
> bug, but at the same time, why would you use "Content-Disposition:
> inline" when you don't have an actual multi-part email? So I do blame
> quilt too for sending nonsensical headers.
>
> (Yes, yes, I see the "It is permissible to use Content-Disposition on
> the main body" in the RFC. But the RFC also makes it clear that it
> actually matters for how things are presented, so saying "ok, I'll do
> flowed" seems equally insane and equally technically RFC-compliant)
Quilt uses those Content-Disposition headers to preserve the patch
filenames; the full headers look like:
Content-Disposition: inline; filename=foo.patch
Various mail clients use that as the default filename when saving
emails, but there's been at least one other mail client that didn't
handle those headers very well.
Is this a new problem with gmail?
I'm not sure whose workflows would break if we kill those headers
altogether, but maybe we can omit them by default.
Andreas
On Tue, Jun 12, 2018 at 11:52 AM Andreas Grünbacher
<[email protected]> wrote:
>
> Quilt uses those Content-Disposition headers to preserve the patch
> filenames;
That' what I was assuming, but does anybody really care?
If you do things one patch at a time, maybe it's convenient, but then
it doesn't sound like a huge win either.
And if you do a patch-series, then it won't work anyway, and you'd be
saving to an mbox or something. Unless people save patch-series things
one by one, but at that point "convenient" is no longer an issue.
> Is this a new problem with gmail?
I don't know. I've seen it for a while, so it's not *new* new, but it
might be something like "the last few months". I know I saw it back in
March - and it was PeterZ then too. So it may literally be the case
that nobody else has sent me patches with quilt.
(Even if they use quilt to manage patches, maybe they don't _send_
them that way?)
How long has quilt been doing it?
Also, note that I'm not at all sure that it's _just_ the
Content-Disposition: inline
that triggers this. There might be something else in those emails that
triggers it but that's the thing that stands out.
Attached is a png on how it looks on the gmail web interface (the
mobile version is similar - I don't use mobile for _work_, but I do
use it for keeping track of stuff that I want to know about, but don't
need to apply or do something about).
Notice how gmail tries to be helpful and make that "attachment" be
downloadable. It is indeed that. But it also makes it actually
illegible as an email.
> I'm not sure whose workflows would break if we kill those headers
> altogether, but maybe we can omit them by default.
That would be lovely at least for my case.
Linus
On Tue, 12 Jun 2018, Linus Torvalds wrote:
> On Tue, Jun 12, 2018 at 11:52 AM Andreas Grünbacher
> <[email protected]> wrote:
> >
> > Quilt uses those Content-Disposition headers to preserve the patch
> > filenames;
>
> That' what I was assuming, but does anybody really care?
People might have scripts using it but I doubt anyone still relies on that
because git mail never did that inline/filename dance so scripts which
convert mboxes to quilt need to have a mechanism to create filenames
anyway. My personal script uses the subject line to create the filenames
and I never tried to use the inline filename as those filenames are often
enough complete garbage.
> (Even if they use quilt to manage patches, maybe they don't _send_
> them that way?)
Many quilt users still do.
> How long has quilt been doing it?
Forever.
> Also, note that I'm not at all sure that it's _just_ the
>
> Content-Disposition: inline
>
> that triggers this. There might be something else in those emails that
> triggers it but that's the thing that stands out.
It is. There is nothing else in the headers which could cause that. quilt
mail format is pretty simplistic.
> > I'm not sure whose workflows would break if we kill those headers
> > altogether, but maybe we can omit them by default.
>
> That would be lovely at least for my case.
No objections.
Thanks,
tglx
On 2018-06-12 21:43:48 [+0200], Thomas Gleixner wrote:
> > Also, note that I'm not at all sure that it's _just_ the
> >
> > Content-Disposition: inline
> >
> > that triggers this. There might be something else in those emails that
> > triggers it but that's the thing that stands out.
>
> It is. There is nothing else in the headers which could cause that. quilt
> mail format is pretty simplistic.
mutt sets this and the tip-bot, too. The difference is that quilt also
sets the filename parameter. Do the mutt or tip-bot mails look flowed?
> Thanks,
>
> tglx
Sebastian
On Tue, Jun 12, 2018 at 2:54 PM Sebastian Andrzej Siewior
<[email protected]> wrote:
>
> mutt sets this and the tip-bot, too. The difference is that quilt also
> sets the filename parameter. Do the mutt or tip-bot mails look flowed?
No, they look fine.
And yes, the difference looks to be that filename. Looking at the
tipbot emails, I see:
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=UTF-8
Content-Disposition: inline
and the email looks perfectly normal.
So it's not Content-Disposition itself that triggers it, it is indeed
that *together* with filename information, so
Content-Type: text/plain; charset=UTF-8
Content-Disposition: inline; filename=peterz-swait-1.patch
makes gmail go "Oh, I want to help you download this thing", and makes
that "download attachment" widget, and makes the text itself act
flowed.
Yeah, that's just stupid of gmail.
I've sent feedback, but judging by my last attempt to get something
fixed (making the mobile application have a "Plain text" mode), I
expect to be laughed at by some coked-up QA person who thinks "flowed"
is obviously better.
Linus
Linus
On 06/12/2018 03:03 PM, Linus Torvalds wrote:
> On Tue, Jun 12, 2018 at 2:54 PM Sebastian Andrzej Siewior
> <[email protected]> wrote:
>>
>> mutt sets this and the tip-bot, too. The difference is that quilt also
>> sets the filename parameter. Do the mutt or tip-bot mails look flowed?
>
> No, they look fine.
>
> And yes, the difference looks to be that filename. Looking at the
> tipbot emails, I see:
>
> Content-Transfer-Encoding: 8bit
> Content-Type: text/plain; charset=UTF-8
> Content-Disposition: inline
>
> and the email looks perfectly normal.
>
> So it's not Content-Disposition itself that triggers it, it is indeed
> that *together* with filename information, so
>
> Content-Type: text/plain; charset=UTF-8
> Content-Disposition: inline; filename=peterz-swait-1.patch
>
> makes gmail go "Oh, I want to help you download this thing", and makes
> that "download attachment" widget, and makes the text itself act
> flowed.
>
> Yeah, that's just stupid of gmail.
>
> I've sent feedback, but judging by my last attempt to get something
> fixed (making the mobile application have a "Plain text" mode), I
> expect to be laughed at by some coked-up QA person who thinks "flowed"
> is obviously better.
FWIW, Peter's patches also show up in Thunderbird as attachments (at least
for me they do).
--
~Randy
Hi Peter, Linus, Andreas,
On Tue, 12 Jun 2018 19:14:20 +0200, Peter Zijlstra wrote:
> On Tue, Jun 12, 2018 at 09:47:34AM -0700, Linus Torvalds wrote:
>
> > I do note how quilt emails are really hard to read, because that:
> >
> > Content-Disposition: inline
> >
> > makes gmail think it's flowed.
> >
> > Which works horribly badly for patches, surprise surprise.
> >
> > So I really wish quilt wouldn't do that. It does smell like a gmail
> > bug, but at the same time, why would you use "Content-Disposition:
> > inline" when you don't have an actual multi-part email? So I do blame
> > quilt too for sending nonsensical headers.
> >
> > (Yes, yes, I see the "It is permissible to use Content-Disposition on
> > the main body" in the RFC. But the RFC also makes it clear that it
> > actually matters for how things are presented, so saying "ok, I'll do
> > flowed" seems equally insane and equally technically RFC-compliant)
>
> Quilt people, anything that can be done about that?
The purpose of the Content-Disposition header is to let quilt store the
original patch file name, so that the recipient can save the email as a
patch file having the exact same name as the sender was using, to make
communication between developers easier. This is the reason why the
header is being added despite the email not being multi-part. As Linus
found out already, RFC 2183 allows that. The RFC also explicitly allows
the use of a filename parameter for inline parts (see section 2.3.)
Using "attachment" instead of "inline" would presumably force the user
to save the patch to a file before being able to read it, or, at least,
to take additional actions in the MUA to convince it to display the
contents inline regardless of what the Content-Disposition header
says. This is clearly not desirable.
We could try specifying the filename directly, without the "inline"
keyword, however that would no longer comply with the RFC
("disposition-parm" is optional, but "disposition-type" is mandatory)
and I am afraid that some MUA implementations would either default to
disposition-type "attachment" in this case, or ignore the header
altogether.
I'm not sure I understand what "flowed" means in this context. If you
mean that gmail breaks the formatting of the patch, I would say that
gmail is infringing the RFC, which clearly stipulates at the beginning
that the Content-Disposition header field is only about telling the MUA
which parts should be displayed immediately and which parts should not,
and not about presentation issues.
Considering that "inline" is the default for a non-multi-part message,
any MUA which changes its behavior in the presence of
"Content-Disposition: inline" is bugged in my opinion.
--
Jean Delvare
SUSE L3 Support
On Tue, 12 Jun 2018 12:23:26 -0700, Linus Torvalds wrote:
> On Tue, Jun 12, 2018 at 11:52 AM Andreas Grünbacher
> <[email protected]> wrote:
> >
> > Quilt uses those Content-Disposition headers to preserve the patch
> > filenames;
>
> That' what I was assuming, but does anybody really care?
Long ago (probably a decade by now, literally) I wrote a shell script
named "rename-patch" for Greg KH which suggests a file name for a patch
received by e-mail. The script first looks for a "filename" attribute
in the Content-Disposition header, and only if not found, falls back to
a heuristic which attempts to generate a good-looking file name based
on the e-mail's subject.
The script used to be published on my kernel.org personal web space,
but went away when kernel.org got hacked, and I never bothered
publishing my few scripts again, sorry about that.
I'm still using that script myself, to name patches generated with "git
show --pretty=email", however there is no Content-Disposition header
there, so the subject heuristic is always used. I don't know if Greg is
still using rename-patch in combination with quilt. Greg?
> If you do things one patch at a time, maybe it's convenient, but then
> it doesn't sound like a huge win either.
>
> And if you do a patch-series, then it won't work anyway, and you'd be
> saving to an mbox or something. Unless people save patch-series things
> one by one, but at that point "convenient" is no longer an issue.
I'm not sure why it wouldn't work with a series. The name information
is available in each patch of the series, and I know that some kernel
developers have all sorts of shortcuts and macros implemented on top of
their MUA to automate queuing of patches for various testing or
publishing purposes.
--
Jean Delvare
SUSE L3 Support
Jean,
2018-06-13 14:32 GMT+02:00 Jean Delvare <[email protected]>:
> Hi Peter, Linus, Andreas,
>
> On Tue, 12 Jun 2018 19:14:20 +0200, Peter Zijlstra wrote:
>> On Tue, Jun 12, 2018 at 09:47:34AM -0700, Linus Torvalds wrote:
>>
>> > I do note how quilt emails are really hard to read, because that:
>> >
>> > Content-Disposition: inline
>> >
>> > makes gmail think it's flowed.
>> >
>> > Which works horribly badly for patches, surprise surprise.
>> >
>> > So I really wish quilt wouldn't do that. It does smell like a gmail
>> > bug, but at the same time, why would you use "Content-Disposition:
>> > inline" when you don't have an actual multi-part email? So I do blame
>> > quilt too for sending nonsensical headers.
>> >
>> > (Yes, yes, I see the "It is permissible to use Content-Disposition on
>> > the main body" in the RFC. But the RFC also makes it clear that it
>> > actually matters for how things are presented, so saying "ok, I'll do
>> > flowed" seems equally insane and equally technically RFC-compliant)
>>
>> Quilt people, anything that can be done about that?
>
> The purpose of the Content-Disposition header is to let quilt store the
> original patch file name, so that the recipient can save the email as a
> patch file having the exact same name as the sender was using, to make
> communication between developers easier. This is the reason why the
> header is being added despite the email not being multi-part. As Linus
> found out already, RFC 2183 allows that. The RFC also explicitly allows
> the use of a filename parameter for inline parts (see section 2.3.)
>
> Using "attachment" instead of "inline" would presumably force the user
> to save the patch to a file before being able to read it, or, at least,
> to take additional actions in the MUA to convince it to display the
> contents inline regardless of what the Content-Disposition header
> says. This is clearly not desirable.
>
> We could try specifying the filename directly, without the "inline"
> keyword, however that would no longer comply with the RFC
> ("disposition-parm" is optional, but "disposition-type" is mandatory)
> and I am afraid that some MUA implementations would either default to
> disposition-type "attachment" in this case, or ignore the header
> altogether.
>
> I'm not sure I understand what "flowed" means in this context. If you
> mean that gmail breaks the formatting of the patch, I would say that
> gmail is infringing the RFC, which clearly stipulates at the beginning
> that the Content-Disposition header field is only about telling the MUA
> which parts should be displayed immediately and which parts should not,
> and not about presentation issues.
>
> Considering that "inline" is the default for a non-multi-part message,
> any MUA which changes its behavior in the presence of
> "Content-Disposition: inline" is bugged in my opinion.
All of that may be correct, but those headers apparently do break
email based patch reviewing on Thunderbird and Gmail now, and that's
not very likely to change. If we continue with our current practice,
we'll end up frustrating users. On top of that, i we make this an
optional feature, quilt users may think that using that option is a
good idea when they will actually break their recipients' workflows.
As Thomas Gleixner wrote in the other thread, most recipients will
already have a way to deal with messages from other sources that don't
include patch filenames, so let's just get rid of Content-Disposition
headers in quilt for good.
Andreas
On Wed, Jun 13, 2018 at 03:00:25PM +0200, Jean Delvare wrote:
> On Tue, 12 Jun 2018 12:23:26 -0700, Linus Torvalds wrote:
> > On Tue, Jun 12, 2018 at 11:52 AM Andreas Gr?nbacher
> > <[email protected]> wrote:
> > >
> > > Quilt uses those Content-Disposition headers to preserve the patch
> > > filenames;
> >
> > That' what I was assuming, but does anybody really care?
>
> Long ago (probably a decade by now, literally) I wrote a shell script
> named "rename-patch" for Greg KH which suggests a file name for a patch
> received by e-mail. The script first looks for a "filename" attribute
> in the Content-Disposition header, and only if not found, falls back to
> a heuristic which attempts to generate a good-looking file name based
> on the e-mail's subject.
>
> The script used to be published on my kernel.org personal web space,
> but went away when kernel.org got hacked, and I never bothered
> publishing my few scripts again, sorry about that.
>
> I'm still using that script myself, to name patches generated with "git
> show --pretty=email", however there is no Content-Disposition header
> there, so the subject heuristic is always used. I don't know if Greg is
> still using rename-patch in combination with quilt. Greg?
Yes I am, I also use it for other things, it's quite useful to me. It's
attached below if anyone else wants it.
But I don't really use the Content-Disposition portion of the logic in
that script much, if any, anymore as I handle most of my normal kernel
work using git. I only use quilt these days for local work before using
git, and for all of my stable kernel work.
So if that logic goes away in quilt, I'm not going to miss it :)
thanks,
greg k-h
On Wed, Jun 13, 2018 at 6:27 AM Andreas Grünbacher
<[email protected]> wrote:
>
> All of that may be correct, but those headers apparently do break
> email based patch reviewing on Thunderbird and Gmail now, and that's
> not very likely to change.
I do want to point out that at least for me, it's not a very big
annoyance. It would be sad if people who actually_use_ quilt lose a
feature they use for this.
I've seen it twice in the last few months, so it's not like it's all
that noticeable. It's a bit annoying when it happens, but it's not a
show-stopper.
Either I don't interact with a lot of quilt users, or (quite likely)
people don't always use quilt itself to then send the patches (the
same way I don't use git send-email to send patches even though I
obviously use git).
Linus
On Wed, 13 Jun 2018 15:27:31 +0200, Andreas Grünbacher wrote:
> All of that may be correct, but those headers apparently do break
> email based patch reviewing on Thunderbird and Gmail now, and that's
> not very likely to change. If we continue with our current practice,
> we'll end up frustrating users. On top of that, i we make this an
> optional feature, quilt users may think that using that option is a
> good idea when they will actually break their recipients' workflows.
> As Thomas Gleixner wrote in the other thread, most recipients will
> already have a way to deal with messages from other sources that don't
> include patch filenames, so let's just get rid of Content-Disposition
> headers in quilt for good.
It always feels bad to me to work around an issue in one piece of
software when the bug is clearly in another piece of software. I'm not
sure why fixes on the correct side of the problem should be unlikely.
The behavior is clearly broken and against the RFC, and there may be
other sources than quilt triggering the bug. Thunderbird is open
source, the problem is identified, it shouldn't be that hard to fix it.
Gmail is a different story, of course, but I guess there are some
developers maintaining it too.
That being said... I agree that recipients of such e-mails most likely
already have an alternative solution in place for non-quilt sources, so
probably removing the Content-Disposition header is not going to cause
too much trouble. So feel free to go ahead and remove it if you think
this is the best thing to do. If some users complain about the change,
I'll let you deal with them ;-)
--
Jean Delvare
SUSE L3 Support
On Tue, Jun 12, 2018 at 10:34:49AM +0200, Peter Zijlstra wrote:
> As mentioned by Linus, swait is exclusive mode and had better behave like it
> and be named like it.
>
> Make it so.
Cool!
When I tried testing it on x86 against recent mainline, I got:
"WARNING: WARNING: Bad or missing .orc_unwind table. Disabling unwinder."
But when I tested on recent mainline without your patches:
4597fcff0704 "Merge tag 'for-4.18/dm-changes-v2' of
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm"
I get the same warning. Trying again on today's mainline, but in the
meantime, is anyone else seeing this?
Thanx, Paul
> ---
> arch/mips/kvm/mips.c | 4 ++--
> arch/powerpc/kvm/book3s_hv.c | 6 +++---
> arch/s390/kvm/interrupt.c | 2 +-
> arch/x86/kernel/kvm.c | 4 ++--
> arch/x86/kvm/lapic.c | 2 +-
> include/linux/swait.h | 36 ++++++++++++++++++------------------
> kernel/power/suspend.c | 4 ++--
> kernel/rcu/srcutiny.c | 4 ++--
> kernel/rcu/tree.c | 8 ++++----
> kernel/rcu/tree_exp.h | 4 ++--
> kernel/rcu/tree_plugin.h | 12 ++++++------
> kernel/sched/swait.c | 32 ++++++++++++++++++++++----------
> virt/kvm/arm/arm.c | 4 ++--
> virt/kvm/arm/psci.c | 2 +-
> virt/kvm/async_pf.c | 2 +-
> virt/kvm/kvm_main.c | 4 ++--
> 16 files changed, 71 insertions(+), 59 deletions(-)
>
>
On Wed, Jun 13, 2018 at 06:27:42PM -0700, Paul E. McKenney wrote:
> On Tue, Jun 12, 2018 at 10:34:49AM +0200, Peter Zijlstra wrote:
> > As mentioned by Linus, swait is exclusive mode and had better behave like it
> > and be named like it.
> >
> > Make it so.
>
> Cool!
>
> When I tried testing it on x86 against recent mainline, I got:
>
> "WARNING: WARNING: Bad or missing .orc_unwind table. Disabling unwinder."
>
> But when I tested on recent mainline without your patches:
>
> 4597fcff0704 "Merge tag 'for-4.18/dm-changes-v2' of
> git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm"
>
> I get the same warning. Trying again on today's mainline, but in the
> meantime, is anyone else seeing this?
And this patch fixes this issue, FWIW:
https://www.mail-archive.com/[email protected]/msg1713797.html
Thanx, Paul