* Re: [PATCH v2 0/7] CLONE_FD: Task exit notification via file descriptor
From: Florian Weimer @ 2015-06-15 10:06 UTC (permalink / raw)
To: Thiago Macieira
Cc: Josh Triplett, Al Viro, Andrew Morton, Andy Lutomirski,
Ingo Molnar, Kees Cook, Oleg Nesterov, Paul E. McKenney,
H. Peter Anvin, Rik van Riel, Thomas Gleixner, Michael Kerrisk,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1775865.3maNSqUW4u@tjmaciei-mobl4>
On 05/29/2015 10:27 PM, Thiago Macieira wrote:
>> It has been suggested (e.g.,
>> <https://sourceware.org/bugzilla/show_bug.cgi?id=15661#c3>) that you can
>> use the existing clone(2) without specifying SIGCHLD to create a new
>> process. The resulting child process is not supposed to show up in
>> wait(2), only in a waitpid(2) (or similar) explicitly specifying the
>> PID. Is this not the case?
>
> Hi Florian
>
> That sounds orthogonal to what we're looking for. Our objective is to get
> notification of when the child exited without resorting to SIGCHLD. If we use
> the regular clone(2) without SIGCHLD and without CLONE_FD, we get no
> notification. The only way to know of the child's termination is by a blocking
> waitpid(2), like you indicated, which is counter productive to our needs.
>
> We need something we can select(2)/poll(2) on.
Thanks for the clarification. I agree that this is a separate and quite
sensible use case.
--
Florian Weimer / Red Hat Product Security
^ permalink raw reply
* Re: [RFC PATCH 02/18] kthread: Add API for iterant kthreads
From: Petr Mladek @ 2015-06-15 12:46 UTC (permalink / raw)
To: Tejun Heo
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150609062349.GW21465-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Tue 2015-06-09 15:23:49, Tejun Heo wrote:
> Hello, Petr.
>
> On Fri, Jun 05, 2015 at 05:01:01PM +0200, Petr Mladek wrote:
> > +static int kthread_iterant_fn(void *kti_ptr)
> > +{
> > + struct kthread_iterant *kti = kti_ptr;
> > + void *data = kti->data;
> > +
> > + if (kti->init)
> > + kti->init(data);
> > +
> > + do {
> > + if (kti->func)
> > + kti->func(data);
>
> Is supporting kthread_iterant w/o the body function intentional? If
> so, did you have anything specific on mind for it? I don't think it
> matters either way. Just curious how this came to be.
Good question. It might make sense to add a warning for kthreads
with empty main function.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH 06/18] signal/kthread: Initial implementation of kthread signal handling
From: Petr Mladek @ 2015-06-15 13:13 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Tejun Heo, Ingo Molnar, Peter Zijlstra,
Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150608211336.GB24869-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Oleg,
I am sorry for the late reply. I wanted to think more before answering
all the mails.
On Mon 2015-06-08 23:13:36, Oleg Nesterov wrote:
> I do not. Contrary, I think this needs more code in the likely case.
> Anyway, this API won't have too many users, so I don't even this this
> is that important.
>
> > > > + if (sig_kernel_stop(signr)) {
> > > > + __set_current_state(TASK_STOPPED);
> > > > + spin_unlock_irqrestore(&sighand->siglock, flags);
> > > > + /* Don't run again until woken by SIGCONT or SIGKILL */
> > > > + freezable_schedule();
> > > > + goto relock;
> > >
> > > Yes this avoids the race with SIGCONT. But as I said we can add another
> > > trivial helper which checks JOBCTL_STOP_DEQUEUED. So a kthread can do
> > > this itself.
> >
> > Hmm, the helper would have a strange semantic. You need to take
> > sighand->siglock, dequeue the signal (SIGSTOP), and call
> > __set_current_state(TASK_STOPPED) before you release the lock.
> > But what would happen if the dequeued signal is _not_ SIGSTOP?
>
> Perhaps I missed your point, but no. If you want to handle SIGSTOP
> you can do
>
I think that we need to add:
spin_lock_irq(&sighand->siglock);
> signr = kthread_signal_dequeue();
> switch (signr) {
> case SIGSTOP:
> something_else();
> kthread_do_signal_stop();
> ...
> }
And if we want to avoid any race, kthread_do_signal_stop() should look like:
void kthread_do_signal_stop(unsigned long flags)
{
struct sighand_struct *sighand = current->sighand;
__set_current_state(TASK_STOPPED);
spin_unlock_irqrestore(&sighand->siglock, flags);
/* Don't run again until woken by SIGCONT or SIGKILL */
freezable_schedule();
}
It means that we will have spin_lock() in one function and
spin_unlock() in another one. This is what I meant with
the strange semantic. This is why I think that it might be
cleaner to implement some generic kthread_do_signal() or so
and allow to (re)define/add sigactions via callbacks.
Note that I am not aware of any kthread that would use SIGSTOP
non-standard way.
Anyway, I am going to concentrate on the main structure of the kthread
API and will put the controversial signal handling a side for now.
I will get back to it when converting the few kthreads that use
signals. I will think more about your feedback in the meantime.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v4 4/4] Documentation: ABI: /sys/firmware/devicetree/overlays
From: Rob Herring @ 2015-06-15 13:24 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, Greg Kroah-Hartman,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pantelis Antoniou
In-Reply-To: <1434137896-13806-5-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On Fri, Jun 12, 2015 at 2:38 PM, Pantelis Antoniou
<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> Documentation ABI entry for overlays sysfs entries.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
> .../ABI/testing/sysfs-firmware-devicetree-overlays | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>
> diff --git a/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
> new file mode 100644
> index 0000000..be2d28b
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
> @@ -0,0 +1,35 @@
> +What: /sys/firmware/devicetree/overlays/
> +Date: March 2015
> +Contact: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> +Description:
> + This directory contains the applied device tree overlays of
> + the running system, as directories of the overlay id.
> +
> + enable: The master enable switch, by default is 1, and when
> + set to 0 it cannot be re-enabled for security reasons.
> +
> +What: /sys/firmware/devicetree/overlays/<id>
> +Date: March 2015
> +Contact: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> +Description:
> + Each directory represents an applied overlay, containing
> + the following attribute files.
> +
> + The discussion about this switch takes place in:
> + http://comments.gmane.org/gmane.linux.drivers.devicetree/101871
> +
> + Kees Cook:
> + "Coming from the perspective of drawing a bright line between
> + kernel and the root user (which tends to start with disabling
> + kernel module loading), I would say that there at least needs
> + to be a high-level one-way "off" switch for the interface so
> + that systems that have this interface can choose to turn it off
> + during initial boot, etc."
Doesn't this below up above with "enable"?
> +
> + targets: A file containing the list of targets of each overlay
> + with each line containing a target.
We have OF nodes in sysfs now. Would it be more useful if we created
links to the target nodes instead of having a list of names?
> +
> + can_remove: The attribute set to 1 means that the overlay can
> + be removed, while 0 means that the overlay is being
> + overlapped therefore removal is prohibited.
> +
> --
> 1.7.12
>
^ permalink raw reply
* Re: [PATCH v4 4/4] Documentation: ABI: /sys/firmware/devicetree/overlays
From: Pantelis Antoniou @ 2015-06-15 13:26 UTC (permalink / raw)
To: Rob Herring
Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, Greg Kroah-Hartman,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqKm9PtDKaabx22Y8WQRJ+RCxWE9Fj+kO2dCJ7_UzriyzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob,
> On Jun 15, 2015, at 16:24 , Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> On Fri, Jun 12, 2015 at 2:38 PM, Pantelis Antoniou
> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>> Documentation ABI entry for overlays sysfs entries.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> ---
>> .../ABI/testing/sysfs-firmware-devicetree-overlays | 35 ++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>>
>> diff --git a/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>> new file mode 100644
>> index 0000000..be2d28b
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>> @@ -0,0 +1,35 @@
>> +What: /sys/firmware/devicetree/overlays/
>> +Date: March 2015
>> +Contact: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> +Description:
>> + This directory contains the applied device tree overlays of
>> + the running system, as directories of the overlay id.
>> +
>> + enable: The master enable switch, by default is 1, and when
>> + set to 0 it cannot be re-enabled for security reasons.
>> +
>> +What: /sys/firmware/devicetree/overlays/<id>
>> +Date: March 2015
>> +Contact: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> +Description:
>> + Each directory represents an applied overlay, containing
>> + the following attribute files.
>> +
>> + The discussion about this switch takes place in:
>> + http://comments.gmane.org/gmane.linux.drivers.devicetree/101871
>> +
>> + Kees Cook:
>> + "Coming from the perspective of drawing a bright line between
>> + kernel and the root user (which tends to start with disabling
>> + kernel module loading), I would say that there at least needs
>> + to be a high-level one-way "off" switch for the interface so
>> + that systems that have this interface can choose to turn it off
>> + during initial boot, etc."
>
> Doesn't this below up above with "enable”?
Ugh I guess so.
>
>> +
>> + targets: A file containing the list of targets of each overlay
>> + with each line containing a target.
>
> We have OF nodes in sysfs now. Would it be more useful if we created
> links to the target nodes instead of having a list of names?
>
Probably, this interface is merely informational; things get complicated by
the fact that there can be more than one target in each overlay.
>> +
>> + can_remove: The attribute set to 1 means that the overlay can
>> + be removed, while 0 means that the overlay is being
>> + overlapped therefore removal is prohibited.
>> +
>> --
>> 1.7.12
>>
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v4 4/4] Documentation: ABI: /sys/firmware/devicetree/overlays
From: Rob Herring @ 2015-06-15 13:42 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, Greg Kroah-Hartman,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <B474B6B9-0C7C-4A46-B64F-BD76DA4B7F3B-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On Mon, Jun 15, 2015 at 8:26 AM, Pantelis Antoniou
<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> Hi Rob,
>
>> On Jun 15, 2015, at 16:24 , Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> On Fri, Jun 12, 2015 at 2:38 PM, Pantelis Antoniou
>> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>>> Documentation ABI entry for overlays sysfs entries.
>>>
>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> ---
>>> .../ABI/testing/sysfs-firmware-devicetree-overlays | 35 ++++++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>> create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>>> new file mode 100644
>>> index 0000000..be2d28b
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>>> @@ -0,0 +1,35 @@
[...]
>>> +
>>> + targets: A file containing the list of targets of each overlay
>>> + with each line containing a target.
>>
>> We have OF nodes in sysfs now. Would it be more useful if we created
>> links to the target nodes instead of having a list of names?
>>
>
> Probably, this interface is merely informational; things get complicated by
> the fact that there can be more than one target in each overlay.
Right, you would need 'targetN' or perhaps '<node name>' (with a '.N'
for duplicates) as the link names.
If it is informational, then perhaps debugfs should be used instead?
What else if anything do you envision adding here?
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 4/4] Documentation: ABI: /sys/firmware/devicetree/overlays
From: Pantelis Antoniou @ 2015-06-15 14:02 UTC (permalink / raw)
To: Rob Herring
Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, Greg Kroah-Hartman, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org
In-Reply-To: <CAL_JsqLQ2fU1KgN=KprLJmGeJjGxcfX9CZfRpWwd-BUYr7g5qw@mail.gmail.com>
Hi Rob,
> On Jun 15, 2015, at 16:42 , Rob Herring <robherring2@gmail.com> wrote:
>
> On Mon, Jun 15, 2015 at 8:26 AM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> Hi Rob,
>>
>>> On Jun 15, 2015, at 16:24 , Rob Herring <robherring2@gmail.com> wrote:
>>>
>>> On Fri, Jun 12, 2015 at 2:38 PM, Pantelis Antoniou
>>> <pantelis.antoniou@konsulko.com> wrote:
>>>> Documentation ABI entry for overlays sysfs entries.
>>>>
>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>> ---
>>>> .../ABI/testing/sysfs-firmware-devicetree-overlays | 35 ++++++++++++++++++++++
>>>> 1 file changed, 35 insertions(+)
>>>> create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>>>> new file mode 100644
>>>> index 0000000..be2d28b
>>>> --- /dev/null
>>>> +++ b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>>>> @@ -0,0 +1,35 @@
>
> [...]
>
>>>> +
>>>> + targets: A file containing the list of targets of each overlay
>>>> + with each line containing a target.
>>>
>>> We have OF nodes in sysfs now. Would it be more useful if we created
>>> links to the target nodes instead of having a list of names?
>>>
>>
>> Probably, this interface is merely informational; things get complicated by
>> the fact that there can be more than one target in each overlay.
>
> Right, you would need 'targetN' or perhaps '<node name>' (with a '.N'
> for duplicates) as the link names.
>
> If it is informational, then perhaps debugfs should be used instead?
>
I’d rather not pull in debugfs here. It’s informational, but not only for
debugging. I’ll see what I can do with the targets.
> What else if anything do you envision adding here?
>
As far as generic infrastructure properties, I hope nothing else.
However I do intend to use the kobj directory to make the overlay users
‘hang’ properties relevant to their use.
For instance the beaglebone capemanager could put there the compatible
and the resource declaration properties. But that’s going to come later.
> Rob
> —
Regards
— Pantelis
^ permalink raw reply
* Re: [RESEND PATCH V2 0/3] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-06-15 14:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Shuah Khan, Michal Hocko, Michael Kerrisk, linux-alpha,
linux-kernel, linux-mips, linux-parisc, linuxppc-dev, sparclinux,
linux-xtensa, linux-mm, linux-arch, linux-api
In-Reply-To: <20150611123424.4bb07cffd0e5bb146cc92231@linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]
On Thu, 11 Jun 2015, Andrew Morton wrote:
> On Thu, 11 Jun 2015 15:21:30 -0400 Eric B Munson <emunson@akamai.com> wrote:
>
> > > Ditto mlockall(MCL_ONFAULT) followed by munlock(). I'm not sure
> > > that even makes sense but the behaviour should be understood and
> > > tested.
> >
> > I have extended the kselftest for lock-on-fault to try both of these
> > scenarios and they work as expected. The VMA is split and the VM
> > flags are set appropriately for the resulting VMAs.
>
> munlock() should do vma merging as well. I *think* we implemented
> that. More tests for you to add ;)
>
> How are you testing the vma merging and splitting, btw? Parsing
> the profcs files?
The lock-on-fault test now covers VMA splitting and merging by parsing
/proc/self/maps. VMA splitting and merging works as it should with both
MAP_LOCKONFAULT and MCL_ONFAULT.
>
> > > What's missing here is a syscall to set VM_LOCKONFAULT on an
> > > arbitrary range of memory - mlock() for lock-on-fault. It's a
> > > shame that mlock() didn't take a `mode' argument. Perhaps we
> > > should add such a syscall - that would make the mmap flag unneeded
> > > but I suppose it should be kept for symmetry.
> >
> > Do you want such a system call as part of this set? I would need some
> > time to make sure I had thought through all the possible corners one
> > could get into with such a call, so it would delay a V3 quite a bit.
> > Otherwise I can send a V3 out immediately.
>
> I think the way to look at this is to pretend that mm/mlock.c doesn't
> exist and ask "how should we design these features".
>
> And that would be:
>
> - mmap() takes a `flags' argument: MAP_LOCKED|MAP_LOCKONFAULT.
>
> - mlock() takes a `flags' argument. Presently that's
> MLOCK_LOCKED|MLOCK_LOCKONFAULT.
>
> - munlock() takes a `flags' arument. MLOCK_LOCKED|MLOCK_LOCKONFAULT
> to specify which flags are being cleared.
>
> - mlockall() and munlockall() ditto.
>
>
> IOW, LOCKED and LOCKEDONFAULT are treated identically and independently.
>
> Now, that's how we would have designed all this on day one. And I
> think we can do this now, by adding new mlock2() and munlock2()
> syscalls. And we may as well deprecate the old mlock() and munlock(),
> not that this matters much.
>
> *should* we do this? I'm thinking "yes" - it's all pretty simple
> boilerplate and wrappers and such, and it gets the interface correct,
> and extensible.
>
> What do others think?
I am working on V3 which will introduce the new system calls.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH V2 0/3] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-06-15 14:43 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Shuah Khan, Michal Hocko, Michael Kerrisk,
linux-alpha, linux-kernel, linux-mips, linux-parisc, linuxppc-dev,
sparclinux, linux-xtensa, linux-mm, linux-arch, linux-api
In-Reply-To: <557ACAFC.90608@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 3898 bytes --]
On Fri, 12 Jun 2015, Vlastimil Babka wrote:
> On 06/11/2015 09:34 PM, Andrew Morton wrote:
> >On Thu, 11 Jun 2015 15:21:30 -0400 Eric B Munson <emunson@akamai.com> wrote:
> >
> >>>Ditto mlockall(MCL_ONFAULT) followed by munlock(). I'm not sure
> >>>that even makes sense but the behaviour should be understood and
> >>>tested.
> >>
> >>I have extended the kselftest for lock-on-fault to try both of these
> >>scenarios and they work as expected. The VMA is split and the VM
> >>flags are set appropriately for the resulting VMAs.
> >
> >munlock() should do vma merging as well. I *think* we implemented
> >that. More tests for you to add ;)
> >
> >How are you testing the vma merging and splitting, btw? Parsing
> >the profcs files?
> >
> >>>What's missing here is a syscall to set VM_LOCKONFAULT on an
> >>>arbitrary range of memory - mlock() for lock-on-fault. It's a
> >>>shame that mlock() didn't take a `mode' argument. Perhaps we
> >>>should add such a syscall - that would make the mmap flag unneeded
> >>>but I suppose it should be kept for symmetry.
> >>
> >>Do you want such a system call as part of this set? I would need some
> >>time to make sure I had thought through all the possible corners one
> >>could get into with such a call, so it would delay a V3 quite a bit.
> >>Otherwise I can send a V3 out immediately.
> >
> >I think the way to look at this is to pretend that mm/mlock.c doesn't
> >exist and ask "how should we design these features".
> >
> >And that would be:
> >
> >- mmap() takes a `flags' argument: MAP_LOCKED|MAP_LOCKONFAULT.
>
> Note that the semantic of MAP_LOCKED can be subtly surprising:
>
> "mlock(2) fails if the memory range cannot get populated to guarantee
> that no future major faults will happen on the range.
> mmap(MAP_LOCKED) on the other hand silently succeeds even if the
> range was populated only
> partially."
>
> ( from http://marc.info/?l=linux-mm&m=143152790412727&w=2 )
>
> So MAP_LOCKED can silently behave like MAP_LOCKONFAULT. While
> MAP_LOCKONFAULT doesn't suffer from such problem, I wonder if that's
> sufficient reason not to extend mmap by new mlock() flags that can
> be instead applied to the VMA after mmapping, using the proposed
> mlock2() with flags. So I think instead we could deprecate
> MAP_LOCKED more prominently. I doubt the overhead of calling the
> extra syscall matters here?
We could talk about retiring the MAP_LOCKED flag but I suspect that
would get significantly more pushback than adding a new mmap flag.
Likely that the overhead does not matter in most cases, but presumably
there are cases where it does (as we have a MAP_LOCKED flag today).
Even with the proposed new system calls I think we should have the
MAP_LOCKONFAULT for parity with MAP_LOCKED.
>
> >- mlock() takes a `flags' argument. Presently that's
> > MLOCK_LOCKED|MLOCK_LOCKONFAULT.
> >
> >- munlock() takes a `flags' arument. MLOCK_LOCKED|MLOCK_LOCKONFAULT
> > to specify which flags are being cleared.
> >
> >- mlockall() and munlockall() ditto.
> >
> >
> >IOW, LOCKED and LOCKEDONFAULT are treated identically and independently.
> >
> >Now, that's how we would have designed all this on day one. And I
> >think we can do this now, by adding new mlock2() and munlock2()
> >syscalls. And we may as well deprecate the old mlock() and munlock(),
> >not that this matters much.
> >
> >*should* we do this? I'm thinking "yes" - it's all pretty simple
> >boilerplate and wrappers and such, and it gets the interface correct,
> >and extensible.
>
> If the new LOCKONFAULT functionality is indeed desired (I haven't
> still decided myself) then I agree that would be the cleanest way.
Do you disagree with the use cases I have listed or do you think there
is a better way of addressing those cases?
>
> >What do others think?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v4] pagemap: switch to the new format and do some cleanup
From: Mark Williamson @ 2015-06-15 14:57 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Naoya Horiguchi,
Linux API, kernel list, Kirill A. Shutemov
In-Reply-To: <20150615055649.4485.92087.stgit@zurg>
Thanks! No outstanding issues with the patchset, from our side.
Reviewed-by: mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org
On Mon, Jun 15, 2015 at 6:56 AM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>
> This patch removes page-shift bits (scheduled to remove since 3.11) and
> completes migration to the new bit layout. Also it cleans messy macro.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>
> ---
>
> v4: fix misprint PM_PFEAME_BITS -> PM_PFRAME_BITS
> ---
> fs/proc/task_mmu.c | 147 ++++++++++++++++---------------------------------
> tools/vm/page-types.c | 29 +++-------
> 2 files changed, 58 insertions(+), 118 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index f1b9ae8..99fa2ae 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -710,23 +710,6 @@ const struct file_operations proc_tid_smaps_operations = {
> .release = proc_map_release,
> };
>
> -/*
> - * We do not want to have constant page-shift bits sitting in
> - * pagemap entries and are about to reuse them some time soon.
> - *
> - * Here's the "migration strategy":
> - * 1. when the system boots these bits remain what they are,
> - * but a warning about future change is printed in log;
> - * 2. once anyone clears soft-dirty bits via clear_refs file,
> - * these flag is set to denote, that user is aware of the
> - * new API and those page-shift bits change their meaning.
> - * The respective warning is printed in dmesg;
> - * 3. In a couple of releases we will remove all the mentions
> - * of page-shift in pagemap entries.
> - */
> -
> -static bool soft_dirty_cleared __read_mostly;
> -
> enum clear_refs_types {
> CLEAR_REFS_ALL = 1,
> CLEAR_REFS_ANON,
> @@ -887,13 +870,6 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
> if (type < CLEAR_REFS_ALL || type >= CLEAR_REFS_LAST)
> return -EINVAL;
>
> - if (type == CLEAR_REFS_SOFT_DIRTY) {
> - soft_dirty_cleared = true;
> - pr_warn_once("The pagemap bits 55-60 has changed their meaning!"
> - " See the linux/Documentation/vm/pagemap.txt for "
> - "details.\n");
> - }
> -
> task = get_proc_task(file_inode(file));
> if (!task)
> return -ESRCH;
> @@ -961,38 +937,26 @@ typedef struct {
> struct pagemapread {
> int pos, len; /* units: PM_ENTRY_BYTES, not bytes */
> pagemap_entry_t *buffer;
> - bool v2;
> bool show_pfn;
> };
>
> #define PAGEMAP_WALK_SIZE (PMD_SIZE)
> #define PAGEMAP_WALK_MASK (PMD_MASK)
>
> -#define PM_ENTRY_BYTES sizeof(pagemap_entry_t)
> -#define PM_STATUS_BITS 3
> -#define PM_STATUS_OFFSET (64 - PM_STATUS_BITS)
> -#define PM_STATUS_MASK (((1LL << PM_STATUS_BITS) - 1) << PM_STATUS_OFFSET)
> -#define PM_STATUS(nr) (((nr) << PM_STATUS_OFFSET) & PM_STATUS_MASK)
> -#define PM_PSHIFT_BITS 6
> -#define PM_PSHIFT_OFFSET (PM_STATUS_OFFSET - PM_PSHIFT_BITS)
> -#define PM_PSHIFT_MASK (((1LL << PM_PSHIFT_BITS) - 1) << PM_PSHIFT_OFFSET)
> -#define __PM_PSHIFT(x) (((u64) (x) << PM_PSHIFT_OFFSET) & PM_PSHIFT_MASK)
> -#define PM_PFRAME_MASK ((1LL << PM_PSHIFT_OFFSET) - 1)
> -#define PM_PFRAME(x) ((x) & PM_PFRAME_MASK)
> -/* in "new" pagemap pshift bits are occupied with more status bits */
> -#define PM_STATUS2(v2, x) (__PM_PSHIFT(v2 ? x : PAGE_SHIFT))
> -
> -#define __PM_SOFT_DIRTY (1LL)
> -#define __PM_MMAP_EXCLUSIVE (2LL)
> -#define PM_PRESENT PM_STATUS(4LL)
> -#define PM_SWAP PM_STATUS(2LL)
> -#define PM_FILE PM_STATUS(1LL)
> -#define PM_NOT_PRESENT(v2) PM_STATUS2(v2, 0)
> +#define PM_ENTRY_BYTES sizeof(pagemap_entry_t)
> +#define PM_PFRAME_BITS 54
> +#define PM_PFRAME_MASK GENMASK_ULL(PM_PFRAME_BITS - 1, 0)
> +#define PM_SOFT_DIRTY BIT_ULL(55)
> +#define PM_MMAP_EXCLUSIVE BIT_ULL(56)
> +#define PM_FILE BIT_ULL(61)
> +#define PM_SWAP BIT_ULL(62)
> +#define PM_PRESENT BIT_ULL(63)
> +
> #define PM_END_OF_BUFFER 1
>
> -static inline pagemap_entry_t make_pme(u64 val)
> +static inline pagemap_entry_t make_pme(u64 frame, u64 flags)
> {
> - return (pagemap_entry_t) { .pme = val };
> + return (pagemap_entry_t) { .pme = (frame & PM_PFRAME_MASK) | flags };
> }
>
> static int add_to_pagemap(unsigned long addr, pagemap_entry_t *pme,
> @@ -1013,7 +977,7 @@ static int pagemap_pte_hole(unsigned long start, unsigned long end,
>
> while (addr < end) {
> struct vm_area_struct *vma = find_vma(walk->mm, addr);
> - pagemap_entry_t pme = make_pme(PM_NOT_PRESENT(pm->v2));
> + pagemap_entry_t pme = make_pme(0, 0);
> /* End of address space hole, which we mark as non-present. */
> unsigned long hole_end;
>
> @@ -1033,7 +997,7 @@ static int pagemap_pte_hole(unsigned long start, unsigned long end,
>
> /* Addresses in the VMA. */
> if (vma->vm_flags & VM_SOFTDIRTY)
> - pme.pme |= PM_STATUS2(pm->v2, __PM_SOFT_DIRTY);
> + pme = make_pme(0, PM_SOFT_DIRTY);
> for (; addr < min(end, vma->vm_end); addr += PAGE_SIZE) {
> err = add_to_pagemap(addr, &pme, pm);
> if (err)
> @@ -1044,50 +1008,44 @@ out:
> return err;
> }
>
> -static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> +static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm,
> struct vm_area_struct *vma, unsigned long addr, pte_t pte)
> {
> - u64 frame = 0, flags;
> + u64 frame = 0, flags = 0;
> struct page *page = NULL;
> - int flags2 = 0;
>
> if (pte_present(pte)) {
> if (pm->show_pfn)
> frame = pte_pfn(pte);
> - flags = PM_PRESENT;
> + flags |= PM_PRESENT;
> page = vm_normal_page(vma, addr, pte);
> if (pte_soft_dirty(pte))
> - flags2 |= __PM_SOFT_DIRTY;
> + flags |= PM_SOFT_DIRTY;
> } else if (is_swap_pte(pte)) {
> swp_entry_t entry;
> if (pte_swp_soft_dirty(pte))
> - flags2 |= __PM_SOFT_DIRTY;
> + flags |= PM_SOFT_DIRTY;
> entry = pte_to_swp_entry(pte);
> frame = swp_type(entry) |
> (swp_offset(entry) << MAX_SWAPFILES_SHIFT);
> - flags = PM_SWAP;
> + flags |= PM_SWAP;
> if (is_migration_entry(entry))
> page = migration_entry_to_page(entry);
> - } else {
> - if (vma->vm_flags & VM_SOFTDIRTY)
> - flags2 |= __PM_SOFT_DIRTY;
> - *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, flags2));
> - return;
> }
>
> if (page && !PageAnon(page))
> flags |= PM_FILE;
> if (page && page_mapcount(page) == 1)
> - flags2 |= __PM_MMAP_EXCLUSIVE;
> - if ((vma->vm_flags & VM_SOFTDIRTY))
> - flags2 |= __PM_SOFT_DIRTY;
> + flags |= PM_MMAP_EXCLUSIVE;
> + if (vma->vm_flags & VM_SOFTDIRTY)
> + flags |= PM_SOFT_DIRTY;
>
> - *pme = make_pme(PM_PFRAME(frame) | PM_STATUS2(pm->v2, flags2) | flags);
> + return make_pme(frame, flags);
> }
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> - pmd_t pmd, int offset, int pmd_flags2)
> +static pagemap_entry_t thp_pmd_to_pagemap_entry(struct pagemapread *pm,
> + pmd_t pmd, int offset, u64 flags)
> {
> u64 frame = 0;
>
> @@ -1099,15 +1057,16 @@ static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *p
> if (pmd_present(pmd)) {
> if (pm->show_pfn)
> frame = pmd_pfn(pmd) + offset;
> - *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
> - PM_STATUS2(pm->v2, pmd_flags2));
> - } else
> - *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, pmd_flags2));
> + flags |= PM_PRESENT;
> + }
> +
> + return make_pme(frame, flags);
> }
> #else
> -static inline void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> - pmd_t pmd, int offset, int pmd_flags2)
> +static pagemap_entry_t thp_pmd_to_pagemap_entry(struct pagemapread *pm,
> + pmd_t pmd, int offset, u64 flags)
> {
> + return make_pme(0, 0);
> }
> #endif
>
> @@ -1121,18 +1080,16 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> int err = 0;
>
> if (pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
> - int pmd_flags2;
> + u64 flags = 0;
>
> if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(*pmd))
> - pmd_flags2 = __PM_SOFT_DIRTY;
> - else
> - pmd_flags2 = 0;
> + flags |= PM_SOFT_DIRTY;
>
> if (pmd_present(*pmd)) {
> struct page *page = pmd_page(*pmd);
>
> if (page_mapcount(page) == 1)
> - pmd_flags2 |= __PM_MMAP_EXCLUSIVE;
> + flags |= PM_MMAP_EXCLUSIVE;
> }
>
> for (; addr != end; addr += PAGE_SIZE) {
> @@ -1141,7 +1098,7 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>
> offset = (addr & ~PAGEMAP_WALK_MASK) >>
> PAGE_SHIFT;
> - thp_pmd_to_pagemap_entry(&pme, pm, *pmd, offset, pmd_flags2);
> + pme = thp_pmd_to_pagemap_entry(pm, *pmd, offset, flags);
> err = add_to_pagemap(addr, &pme, pm);
> if (err)
> break;
> @@ -1161,7 +1118,7 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> for (; addr < end; pte++, addr += PAGE_SIZE) {
> pagemap_entry_t pme;
>
> - pte_to_pagemap_entry(&pme, pm, vma, addr, *pte);
> + pme = pte_to_pagemap_entry(pm, vma, addr, *pte);
> err = add_to_pagemap(addr, &pme, pm);
> if (err)
> break;
> @@ -1174,19 +1131,18 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> }
>
> #ifdef CONFIG_HUGETLB_PAGE
> -static void huge_pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> - pte_t pte, int offset, int flags2)
> +static pagemap_entry_t huge_pte_to_pagemap_entry(struct pagemapread *pm,
> + pte_t pte, int offset, u64 flags)
> {
> u64 frame = 0;
>
> if (pte_present(pte)) {
> if (pm->show_pfn)
> frame = pte_pfn(pte) + offset;
> - *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
> - PM_STATUS2(pm->v2, flags2));
> - } else
> - *pme = make_pme(PM_NOT_PRESENT(pm->v2) |
> - PM_STATUS2(pm->v2, flags2));
> + flags |= PM_PRESENT;
> + }
> +
> + return make_pme(frame, flags);
> }
>
> /* This function walks within one hugetlb entry in the single call */
> @@ -1197,17 +1153,15 @@ static int pagemap_hugetlb_range(pte_t *pte, unsigned long hmask,
> struct pagemapread *pm = walk->private;
> struct vm_area_struct *vma = walk->vma;
> int err = 0;
> - int flags2;
> + u64 flags = 0;
> pagemap_entry_t pme;
>
> if (vma->vm_flags & VM_SOFTDIRTY)
> - flags2 = __PM_SOFT_DIRTY;
> - else
> - flags2 = 0;
> + flags |= PM_SOFT_DIRTY;
>
> for (; addr != end; addr += PAGE_SIZE) {
> int offset = (addr & ~hmask) >> PAGE_SHIFT;
> - huge_pte_to_pagemap_entry(&pme, pm, *pte, offset, flags2);
> + pme = huge_pte_to_pagemap_entry(pm, *pte, offset, flags);
> err = add_to_pagemap(addr, &pme, pm);
> if (err)
> return err;
> @@ -1228,7 +1182,9 @@ static int pagemap_hugetlb_range(pte_t *pte, unsigned long hmask,
> * Bits 0-54 page frame number (PFN) if present
> * Bits 0-4 swap type if swapped
> * Bits 5-54 swap offset if swapped
> - * Bits 55-60 page shift (page size = 1<<page shift)
> + * Bit 55 pte is soft-dirty (see Documentation/vm/soft-dirty.txt)
> + * Bit 56 page exclusively mapped
> + * Bits 57-60 zero
> * Bit 61 page is file-page or shared-anon
> * Bit 62 page swapped
> * Bit 63 page present
> @@ -1269,7 +1225,6 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>
> /* do not disclose physical addresses: attack vector */
> pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);
> - pm.v2 = soft_dirty_cleared;
> pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
> pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);
> ret = -ENOMEM;
> @@ -1339,10 +1294,6 @@ static int pagemap_open(struct inode *inode, struct file *file)
> {
> struct mm_struct *mm;
>
> - pr_warn_once("Bits 55-60 of /proc/PID/pagemap entries are about "
> - "to stop being page-shift some time soon. See the "
> - "linux/Documentation/vm/pagemap.txt for details.\n");
> -
> mm = proc_mem_open(inode, PTRACE_MODE_READ);
> if (IS_ERR(mm))
> return PTR_ERR(mm);
> diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
> index 3a9f193..e1d5ff8 100644
> --- a/tools/vm/page-types.c
> +++ b/tools/vm/page-types.c
> @@ -57,26 +57,15 @@
> * pagemap kernel ABI bits
> */
>
> -#define PM_ENTRY_BYTES sizeof(uint64_t)
> -#define PM_STATUS_BITS 3
> -#define PM_STATUS_OFFSET (64 - PM_STATUS_BITS)
> -#define PM_STATUS_MASK (((1LL << PM_STATUS_BITS) - 1) << PM_STATUS_OFFSET)
> -#define PM_STATUS(nr) (((nr) << PM_STATUS_OFFSET) & PM_STATUS_MASK)
> -#define PM_PSHIFT_BITS 6
> -#define PM_PSHIFT_OFFSET (PM_STATUS_OFFSET - PM_PSHIFT_BITS)
> -#define PM_PSHIFT_MASK (((1LL << PM_PSHIFT_BITS) - 1) << PM_PSHIFT_OFFSET)
> -#define __PM_PSHIFT(x) (((uint64_t) (x) << PM_PSHIFT_OFFSET) & PM_PSHIFT_MASK)
> -#define PM_PFRAME_MASK ((1LL << PM_PSHIFT_OFFSET) - 1)
> -#define PM_PFRAME(x) ((x) & PM_PFRAME_MASK)
> -
> -#define __PM_SOFT_DIRTY (1LL)
> -#define __PM_MMAP_EXCLUSIVE (2LL)
> -#define PM_PRESENT PM_STATUS(4LL)
> -#define PM_SWAP PM_STATUS(2LL)
> -#define PM_FILE PM_STATUS(1LL)
> -#define PM_SOFT_DIRTY __PM_PSHIFT(__PM_SOFT_DIRTY)
> -#define PM_MMAP_EXCLUSIVE __PM_PSHIFT(__PM_MMAP_EXCLUSIVE)
> -
> +#define PM_ENTRY_BYTES 8
> +#define PM_PFRAME_BITS 54
> +#define PM_PFRAME_MASK ((1LL << PM_PFRAME_BITS) - 1)
> +#define PM_PFRAME(x) ((x) & PM_PFRAME_MASK)
> +#define PM_SOFT_DIRTY (1ULL << 55)
> +#define PM_MMAP_EXCLUSIVE (1ULL << 56)
> +#define PM_FILE (1ULL << 61)
> +#define PM_SWAP (1ULL << 62)
> +#define PM_PRESENT (1ULL << 63)
>
> /*
> * kernel page flags
>
^ permalink raw reply
* Re: [RFC PATCH 15/18] ring_buffer: Allow to exit the ring buffer benchmark immediately
From: Petr Mladek @ 2015-06-15 15:23 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra, Richard Weinberger, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150608134417.3fb7a811-f9ZlEuEWxVcJvu8Pb33WZ0EMvNT87kid@public.gmane.org>
On Mon 2015-06-08 13:44:17, Steven Rostedt wrote:
> On Fri, 5 Jun 2015 17:01:14 +0200
> Petr Mladek <pmladek-AlSwsSmVLrQ@public.gmane.org> wrote:
>
> > It takes a while until the ring_buffer_benchmark module is removed
> > when the ring buffer hammer is running. It is because it takes
> > few seconds and kthread_should_terminate() is not being checked.
> >
> > This patch adds the check for kthread termination into the producer.
> > It uses the existing kill_test flag to finish the kthreads as
> > cleanly as possible.
> >
> > It disables printing the "ERROR" message when the kthread is going.
> >
> > Also it makes sure that producer does not go into the 10sec sleep
> > when it is being killed.
>
> This patch looks like something I may take regardless of the other
> patches (if it applies cleanly).
Please, find below a version of the patch that can be applied against
current Linus tree and also against your for-next branch.
> As for the other patches, the ring buffer benchmark is just that, a
> benchmark that I use when making changes to the ring buffer. It's not
> something for production systems.
I see.
> What about just adding a depend on !LIVE_PATCHING to
> RING_BUFFER_BENCHMARK, or force it to shut down during patching.
> There's no reason to make it safe to be running when you patch the
> kernel. Just adds complexity to some simple code.
I would like to convert all kthreads into some sane API. I hope
that it will make them rather easier. Let's see how it will look
like in the end.
Anyway, the two kthreads in this benchmark are in the "easy" group
regarding patching because they are sleeping only limited time.
>From 7c1f80e8389c036a06f35c5fd94a648025b995be Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek-AlSwsSmVLrQ@public.gmane.org>
Date: Mon, 15 Jun 2015 15:53:10 +0200
Subject: [PATCH] ring_buffer: Allow to exit the ring buffer benchmark
immediately
It takes a while until the ring_buffer_benchmark module is removed
when the ring buffer hammer is running. It is because it takes
few seconds and kthread_should_stop() is not being checked.
This patch adds the check for kthread termination into the producer.
It uses the existing @kill_test flag to finish the kthreads as
cleanly as possible.
It disables printing the "ERROR" message when the kthread is going.
It makes sure that producer does not go into the 10sec sleep
when it is being killed.
Finally, it does not call wait_to_die() when kthread_should_stop()
already returns true.
Signed-off-by: Petr Mladek <pmladek-AlSwsSmVLrQ@public.gmane.org>
---
kernel/trace/ring_buffer_benchmark.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 1b28df2d9104..5b3583a01849 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -263,6 +263,8 @@ static void ring_buffer_producer(void)
if (cnt % wakeup_interval)
cond_resched();
#endif
+ if (kthread_should_stop())
+ kill_test = 1;
} while (ktime_before(end_time, timeout) && !kill_test);
trace_printk("End ring buffer hammer\n");
@@ -285,7 +287,7 @@ static void ring_buffer_producer(void)
entries = ring_buffer_entries(buffer);
overruns = ring_buffer_overruns(buffer);
- if (kill_test)
+ if (kill_test && !kthread_should_stop())
trace_printk("ERROR!\n");
if (!disable_reader) {
@@ -379,7 +381,7 @@ static int ring_buffer_consumer_thread(void *arg)
}
__set_current_state(TASK_RUNNING);
- if (kill_test)
+ if (!kthread_should_stop)
wait_to_die();
return 0;
@@ -399,13 +401,16 @@ static int ring_buffer_producer_thread(void *arg)
}
ring_buffer_producer();
+ if (kill_test)
+ goto out_kill;
trace_printk("Sleeping for 10 secs\n");
set_current_state(TASK_INTERRUPTIBLE);
schedule_timeout(HZ * SLEEP_TIME);
}
- if (kill_test)
+out_kill:
+ if (!kthread_should_stop())
wait_to_die();
return 0;
--
1.8.5.6
^ permalink raw reply related
* Re: [RFC PATCH 15/18] ring_buffer: Allow to exit the ring buffer benchmark immediately
From: Steven Rostedt @ 2015-06-15 15:33 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra, Richard Weinberger, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150615152313.GC3135-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
On Mon, 15 Jun 2015 17:23:13 +0200
Petr Mladek <pmladek-AlSwsSmVLrQ@public.gmane.org> wrote:
> Please, find below a version of the patch that can be applied against
> current Linus tree and also against your for-next branch.
>
Thanks, just on Friday, I was testing ring buffer changes with the
ringbuffer benchmark, and was bitching about how I had to wait the 10
seconds before the module would unload ;-)
-- Steve
^ permalink raw reply
* Re: [RFC PATCH 15/18] ring_buffer: Allow to exit the ring buffer benchmark immediately
From: Petr Mladek @ 2015-06-15 15:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra, Richard Weinberger, David Woodhouse, linux-mtd,
Trond Myklebust, Anna Schumaker, linux-nfs, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko, live-patching, linux-api,
linux-kernel
In-Reply-To: <20150615113343.215bcd43@gandalf.local.home>
On Mon 2015-06-15 11:33:43, Steven Rostedt wrote:
> On Mon, 15 Jun 2015 17:23:13 +0200
> Petr Mladek <pmladek@suse.cz> wrote:
>
>
> > Please, find below a version of the patch that can be applied against
> > current Linus tree and also against your for-next branch.
> >
>
> Thanks, just on Friday, I was testing ring buffer changes with the
> ringbuffer benchmark, and was bitching about how I had to wait the 10
> seconds before the module would unload ;-)
I am glad that it will be useful.
Please, find below a version with a small typo fix. Heh, I spent a lot
of time with doulbe checking the patch and then decided to do a last
minute change. It produced "only" compilation warning that I missed.
It was not visible with my testing :-(
>From 0049043e740c22bddc4306e448a015a9b8450825 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.cz>
Date: Mon, 15 Jun 2015 15:53:10 +0200
Subject: [PATCH] ring_buffer: Allow to exit the ring buffer benchmark
immediately
It takes a while until the ring_buffer_benchmark module is removed
when the ring buffer hammer is running. It is because it takes
few seconds and kthread_should_stop() is not being checked.
This patch adds the check for kthread termination into the producer.
It uses the existing @kill_test flag to finish the kthreads as
cleanly as possible.
It disables printing the "ERROR" message when the kthread is going.
It makes sure that producer does not go into the 10sec sleep
when it is being killed.
Finally, it does not call wait_to_die() when kthread_should_stop()
already returns true.
Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
kernel/trace/ring_buffer_benchmark.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 1b28df2d9104..c17f9c8d4908 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -263,6 +263,8 @@ static void ring_buffer_producer(void)
if (cnt % wakeup_interval)
cond_resched();
#endif
+ if (kthread_should_stop())
+ kill_test = 1;
} while (ktime_before(end_time, timeout) && !kill_test);
trace_printk("End ring buffer hammer\n");
@@ -285,7 +287,7 @@ static void ring_buffer_producer(void)
entries = ring_buffer_entries(buffer);
overruns = ring_buffer_overruns(buffer);
- if (kill_test)
+ if (kill_test && !kthread_should_stop())
trace_printk("ERROR!\n");
if (!disable_reader) {
@@ -379,7 +381,7 @@ static int ring_buffer_consumer_thread(void *arg)
}
__set_current_state(TASK_RUNNING);
- if (kill_test)
+ if (!kthread_should_stop())
wait_to_die();
return 0;
@@ -399,13 +401,16 @@ static int ring_buffer_producer_thread(void *arg)
}
ring_buffer_producer();
+ if (kill_test)
+ goto out_kill;
trace_printk("Sleeping for 10 secs\n");
set_current_state(TASK_INTERRUPTIBLE);
schedule_timeout(HZ * SLEEP_TIME);
}
- if (kill_test)
+out_kill:
+ if (!kthread_should_stop())
wait_to_die();
return 0;
--
1.8.5.6
^ permalink raw reply related
* Re: [RFC PATCH 06/18] signal/kthread: Initial implementation of kthread signal handling
From: Oleg Nesterov @ 2015-06-15 19:14 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Tejun Heo, Ingo Molnar, Peter Zijlstra,
Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150615131341.GN9409-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
Hi Petr,
On 06/15, Petr Mladek wrote:
>
> I am sorry for the late reply. I wanted to think more before answering
> all the mails.
Don't worry I am always late ;)
> On Mon 2015-06-08 23:13:36, Oleg Nesterov wrote:
> > >
> > > Hmm, the helper would have a strange semantic. You need to take
> > > sighand->siglock, dequeue the signal (SIGSTOP), and call
> > > __set_current_state(TASK_STOPPED) before you release the lock.
> > > But what would happen if the dequeued signal is _not_ SIGSTOP?
> >
> > Perhaps I missed your point, but no. If you want to handle SIGSTOP
> > you can do
> >
>
> I think that we need to add:
>
> spin_lock_irq(&sighand->siglock);
>
> > signr = kthread_signal_dequeue();
> > switch (signr) {
> > case SIGSTOP:
> > something_else();
> > kthread_do_signal_stop();
> > ...
> > }
>
> And if we want to avoid any race, kthread_do_signal_stop() should look like:
>
> void kthread_do_signal_stop(unsigned long flags)
> {
> struct sighand_struct *sighand = current->sighand;
>
> __set_current_state(TASK_STOPPED);
> spin_unlock_irqrestore(&sighand->siglock, flags);
> /* Don't run again until woken by SIGCONT or SIGKILL */
> freezable_schedule();
> }
Ah, understand. You think that we need to take ->siglock in advance
to avoid the race with SIGCONT?
No, we don't. Let me show you the code I suggested again:
void kthread_do_signal_stop(void)
{
spin_lock_irq(&curtent->sighand->siglock);
if (current->jobctl & JOBCTL_STOP_DEQUEUED)
__set_current_state(TASK_STOPPED);
spin_unlock_irq(¤t->sighand->siglock);
schedule();
}
so you can dequeue_signal() and call kthread_do_signal_stop() without
holding ->siglock. We can rely on JOBCTL_STOP_DEQUEUED bit. SIGCONT
clears it, so kthread_do_signal_stop() can't race.
Oleg.
^ permalink raw reply
* Re: [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Kees Cook @ 2015-06-15 20:19 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tycho Andersen, LKML, Linux API, Will Drewry, Roland McGrath,
Pavel Emelyanov, Serge E. Hallyn, Andy Lutomirski
In-Reply-To: <20150613235250.GA25252-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Sat, Jun 13, 2015 at 4:52 PM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 06/13, Tycho Andersen wrote:
>>
>> This patch is the first step in enabling checkpoint/restore of processes
>> with seccomp enabled.
>
> So just in case, I am fine with this version.
Should I add your Ack? Though I really like the idea of a
"Fine-with-this:" tag. ;)
Andy, if you're okay with this too, I'll add it to my seccomp tree.
Thanks again Tycho!
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Andy Lutomirski @ 2015-06-15 20:50 UTC (permalink / raw)
To: Kees Cook
Cc: Oleg Nesterov, Tycho Andersen, LKML, Linux API, Will Drewry,
Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <CAGXu5jJt93GKadDonjgYkrjh2Z44QyyzxmJwNOt1MVNM9gekJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Jun 15, 2015 at 1:19 PM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Sat, Jun 13, 2015 at 4:52 PM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On 06/13, Tycho Andersen wrote:
>>>
>>> This patch is the first step in enabling checkpoint/restore of processes
>>> with seccomp enabled.
>>
>> So just in case, I am fine with this version.
>
> Should I add your Ack? Though I really like the idea of a
> "Fine-with-this:" tag. ;)
>
> Andy, if you're okay with this too, I'll add it to my seccomp tree.
I'm fine with this. :)
Acked-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Thanks again Tycho!
>
> -Kees
>
> --
> Kees Cook
> Chrome OS Security
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Oleg Nesterov @ 2015-06-15 21:20 UTC (permalink / raw)
To: Kees Cook
Cc: Tycho Andersen, LKML, Linux API, Will Drewry, Roland McGrath,
Pavel Emelyanov, Serge E. Hallyn, Andy Lutomirski
In-Reply-To: <CAGXu5jJt93GKadDonjgYkrjh2Z44QyyzxmJwNOt1MVNM9gekJQ@mail.gmail.com>
On 06/15, Kees Cook wrote:
>
> On Sat, Jun 13, 2015 at 4:52 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> > On 06/13, Tycho Andersen wrote:
> >>
> >> This patch is the first step in enabling checkpoint/restore of processes
> >> with seccomp enabled.
> >
> > So just in case, I am fine with this version.
>
> Should I add your Ack? Though I really like the idea of a
> "Fine-with-this:" tag. ;)
Yes, please feel free to add ;)
Oleg.
^ permalink raw reply
* Re: [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Kees Cook @ 2015-06-15 22:04 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tycho Andersen, LKML, Linux API, Will Drewry, Roland McGrath,
Pavel Emelyanov, Serge E. Hallyn, Andy Lutomirski
In-Reply-To: <20150615212017.GA10538@redhat.com>
On Mon, Jun 15, 2015 at 2:20 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 06/15, Kees Cook wrote:
>>
>> On Sat, Jun 13, 2015 at 4:52 PM, Oleg Nesterov <oleg@redhat.com> wrote:
>> > On 06/13, Tycho Andersen wrote:
>> >>
>> >> This patch is the first step in enabling checkpoint/restore of processes
>> >> with seccomp enabled.
>> >
>> > So just in case, I am fine with this version.
>>
>> Should I add your Ack? Though I really like the idea of a
>> "Fine-with-this:" tag. ;)
>
> Yes, please feel free to add ;)
Thanks!
I've added this to the seccomp tree. It may be a bit delayed appearing
in -next, due to 4.2 opening soon.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Kees Cook @ 2015-06-15 22:50 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tycho Andersen, LKML, Linux API, Will Drewry, Roland McGrath,
Pavel Emelyanov, Serge E. Hallyn, Andy Lutomirski
In-Reply-To: <CAGXu5jK9XA6hpjvOma647aKmLL7q89UJhQy_0BB90wUSPNT0hQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Jun 15, 2015 at 3:04 PM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Mon, Jun 15, 2015 at 2:20 PM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On 06/15, Kees Cook wrote:
>>>
>>> On Sat, Jun 13, 2015 at 4:52 PM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> > On 06/13, Tycho Andersen wrote:
>>> >>
>>> >> This patch is the first step in enabling checkpoint/restore of processes
>>> >> with seccomp enabled.
>>> >
>>> > So just in case, I am fine with this version.
>>>
>>> Should I add your Ack? Though I really like the idea of a
>>> "Fine-with-this:" tag. ;)
>>
>> Yes, please feel free to add ;)
>
> Thanks!
>
> I've added this to the seccomp tree. It may be a bit delayed appearing
> in -next, due to 4.2 opening soon.
>
> -Kees
Tycho, would you be willing to send some man-page updates for this new
interface, so it's documented correctly for ptrace?
https://www.kernel.org/doc/man-pages/patches.html
Michael, see I thought about linux-api before it was actually in
Linus's tree! ;) If you see any issues here, please let us know.
Otherwise, hopefully Tycho will get you some updates to ptrace.2
and/or seccomp.2. :)
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH v2 net-next 0/3] bpf: share helpers between tracing and networking
From: David Miller @ 2015-06-15 23:01 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: luto-kltTT9wpgjJwATOyAt5JVQ, mingo-DgEjT+Ai2ygdnm+yROfE0A,
rostedt-nx8X9YLhiw1AfugRpC6u6w, wangnan0-hv44wF8Li93QT0dZR+AlfA,
lizefan-hv44wF8Li93QT0dZR+AlfA,
daniel.wagner-98C5kh4wR6ohFhg+JK9F0w,
daniel-FeC+5ew28dpmcu3hnIyYJQ, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434163154-5218-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Fri, 12 Jun 2015 19:39:11 -0700
> v1->v2: switched to init_user_ns from current_user_ns as suggested by Andy
>
> Introduce new helpers to access 'struct task_struct'->pid, tgid, uid, gid, comm
> fields in tracing and networking.
>
> Share bpf_trace_printk() and bpf_get_smp_processor_id() helpers between
> tracing and networking.
Series applied, thanks.
Although I agree with the sentiment that this thing can cause
surprising results and can be asking for trouble.
If someone wants to filter traffic "by UID" they might make
a simple ingress TC ebpf program using these new interfaces
and expect it to work.
But the UID their program will see will be the UID of whatever
randomly happened to be executing when the packet was received
and processed.
So for these kinds of things such identifying markers are less
useful, and perhaps surprising in their results.
^ permalink raw reply
* Re: [PATCH] include/uapi/linux/swab.h: define a silent macro to avoid sparse error
From: Greg KH @ 2015-06-15 23:06 UTC (permalink / raw)
To: Bilel DRIRA
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434320613-28700-1-git-send-email-bilel.dr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Sun, Jun 14, 2015 at 11:23:32PM +0100, Bilel DRIRA wrote:
> define a silent macro when _CHECKER_ is defined.
> This change fixes the following sparse errors:
>
> include/uapi/linux/swab.h:60:16: error: undefined identifier '__builtin_bswap32'
> include/uapi/linux/swab.h:60:33: error: not a function <noident>
> include/uapi/linux/swab.h:71:16: error: undefined identifier '__builtin_bswap64'
> include/uapi/linux/swab.h:71:33: error: not a function <noident>
> include/uapi/linux/swab.h:60:33: error: not a function <noident>
>
> Signed-off-by: Bilel DRIRA <bilel.dr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> include/uapi/linux/swab.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
> index 0e011eb..c04de49 100644
> --- a/include/uapi/linux/swab.h
> +++ b/include/uapi/linux/swab.h
> @@ -5,6 +5,18 @@
> #include <linux/compiler.h>
> #include <asm/swab.h>
>
> +#ifdef __CHECKER__
> +
> +#ifdef __HAVE_BUILTIN_BSWAP64__
> +#define __builtin_bswap64(val) (0)
> +#endif
> +
> +#ifdef __HAVE_BUILTIN_BSWAP32__
> +#define __builtin_bswap32(val) (0)
> +#endif
> +
> +#endif /* __CHECKER__ */
Shouldn't the tool be fixed instead of papering over the issue with
random defines like this?
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC v2 23/24] m68k/mac: Fix PRAM accessors
From: Finn Thain @ 2015-06-16 3:10 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux/m68k,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdWzKvWjAE3u54E4xewEDX+zJqEDEvRmO2twpeD2vmBzog-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, 15 Jun 2015, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Sun, Jun 14, 2015 at 9:46 AM, Finn Thain <fthain-PnMVE5gNl/W9dxLkCovUHLpzq4S04n8Q@public.gmane.org> wrote:
> > --- linux.orig/arch/m68k/mac/misc.c 2015-06-14 17:46:02.000000000 +1000
> > +++ linux/arch/m68k/mac/misc.c 2015-06-14 17:46:03.000000000 +1000
> > @@ -284,11 +287,31 @@ static void via_pram_command(int command
> >
> > static unsigned char via_pram_read_byte(int offset)
> > {
> > - return 0;
> > + unsigned char temp;
> > + int addr = ((offset & 0xE0) << 3) | ((offset & 0x1F) << 2);
>
> Can you please add #defines for the magic values?
This is just marshalling the offset argument. I don't know how to rewrite
that code more clearly. I'm open to suggestions. I don't know of any
documentation that gives names or meanings to the different bit ranges. I
infered the format from the MESS source code,
https://github.com/mamedev/mame/blob/master/src/mess/machine/macrtc.c
What I found in the MESS source code looks like the result of reverse
engineering. Hence the RTC code in this patch also looks like a reverse
engineered driver.
>
> > +
> > + /* Use RTC command 0x38 for XPRAM access, as per MESS source code */
> > + via_pram_command(addr | 0x3800 | 0x8001, &temp);
>
> It seems 0x38 is already documented in <linux/pmu.h> (see below), or not
> (it's shifted left by 8 bits?)?
No, this is a RTC command not a PMU command. This RTC device is an Apple
custom IC that is publicly undocumented. OTOH, the PMU device is well
documented, since Apple publicly released PMU driver source code in
MkLinux and later in XNU. That's why I've been able to provide #defines
for the PMU commands but not the RTC commands.
>
> > +
> > + return temp;
> > }
> >
> > static void via_pram_write_byte(unsigned char data, int offset)
> > {
> > + unsigned char temp;
> > + int addr = ((offset & 0xE0) << 3) | ((offset & 0x1F) << 2);
> > +
> > + /* Clear the write protect bit */
> > + temp = 0x55;
> > + via_pram_command(0x34 | 0x01, &temp);
> > +
> > + /* Write the byte to XPRAM */
> > + temp = data;
> > + via_pram_command(0x3800 | 0x0001 | addr, &temp);
> > +
> > + /* Set the write protect bit */
> > + temp = 0xD5;
> > + via_pram_command(0x34 | 0x01, &temp);
>
> More magic values...
When I have reliable documentation I always define macros. So I agree that
"command" bytes like 0x34 and 0x3800 should have names but what are the
correct names? Are we constructing an opcode containing RTC register file
addresses or are we issuing read/write accesses to chip registers?
In my experience with undocumented 68k Mac hardware and its Linux port,
codified guesswork is worse than no documentation at all. The only useful
RTC documentation I've ever come across is this:
http://mac.linux-m68k.org/devel/plushw.php
It tells us that the two least significant bits bits must equal 0b01. What
would you call that macro? It also tells us that the most significant bit,
0x80, means "read access" but it only mentions early RTC chips and so it
does not cover two byte opcodes. Should I have used 0x8080 here?
Whatever your opinion of reverse engineered drivers, the changes in this
patch are consistent with the rest of the file. E.g. via_read_time() and
via_write_time(). If/when we have the chip data needed to correctly define
macros for 0x01, 0x0001, 0x80, 0x8000 or 0x8080, I think they should be
applied across the entire file, and in a different patch. Inconsistent use
of such macros would be undesirable IMHO.
>
> > }
> >
> > /*
> > Index: linux/include/uapi/linux/pmu.h
> > ===================================================================
> > --- linux.orig/include/uapi/linux/pmu.h 2015-06-14 17:45:34.000000000 +1000
> > +++ linux/include/uapi/linux/pmu.h 2015-06-14 17:46:03.000000000 +1000
> > @@ -18,7 +18,9 @@
> > #define PMU_POWER_CTRL 0x11 /* control power of some devices */
> > #define PMU_ADB_CMD 0x20 /* send ADB packet */
> > #define PMU_ADB_POLL_OFF 0x21 /* disable ADB auto-poll */
> > +#define PMU_WRITE_XPRAM 0x32 /* write eXtended Parameter RAM */
> > #define PMU_WRITE_NVRAM 0x33 /* write non-volatile RAM */
> > +#define PMU_READ_XPRAM 0x3a /* read eXtended Parameter RAM */
> > #define PMU_READ_NVRAM 0x3b /* read non-volatile RAM */
> > #define PMU_SET_RTC 0x30 /* set real-time clock */
> > #define PMU_READ_RTC 0x38 /* read real-time clock */
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
^ permalink raw reply
* Re: [PATCH v2 net-next 0/3] bpf: share helpers between tracing and networking
From: Alexei Starovoitov @ 2015-06-16 3:28 UTC (permalink / raw)
To: David Miller
Cc: luto-kltTT9wpgjJwATOyAt5JVQ, mingo-DgEjT+Ai2ygdnm+yROfE0A,
rostedt-nx8X9YLhiw1AfugRpC6u6w, wangnan0-hv44wF8Li93QT0dZR+AlfA,
lizefan-hv44wF8Li93QT0dZR+AlfA,
daniel.wagner-98C5kh4wR6ohFhg+JK9F0w,
daniel-FeC+5ew28dpmcu3hnIyYJQ, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150615.160130.583783771772303463.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On 6/15/15 4:01 PM, David Miller wrote:
>
> Although I agree with the sentiment that this thing can cause
> surprising results and can be asking for trouble.
>
> If someone wants to filter traffic "by UID" they might make
> a simple ingress TC ebpf program using these new interfaces
> and expect it to work.
>
> But the UID their program will see will be the UID of whatever
> randomly happened to be executing when the packet was received
> and processed.
yes, you're right. Such tc filters will be incorrect.
Will send a partial revert disallowing them in tc.
^ permalink raw reply
* Re: [PATCH] ipc: Modify message queue accounting to reflect both total user data and auxiliary kernel data
From: Michael Kerrisk (man-pages) @ 2015-06-16 7:36 UTC (permalink / raw)
To: Marcus Gelderie, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w,
dhowells-H+wXaHxf7aLQT0dZR+AlfA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
dledford-H+wXaHxf7aLQT0dZR+AlfA, John Duffy, Arto Bendiken,
linux-api-u79uwXL29TY76Z2rM5mHXA, Davidlohr Bueso
In-Reply-To: <20150613194039.GB31082-W7fNxlbxG8VSq9BJjBFyUp/QNRX+jHPU@public.gmane.org>
[Added a couple of people into CC that I know are interested/participated
in past discussions that were related. Also added linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org]
Background for those new to the topic:
The pseudofiles in the mqueue filesystem (usually mounted at
/dev/mqueue) expose fields with metadata describing a message
queue. One of these fields, QSIZE, as originally implemented,
showed the total number of bytes of user data in all messages in
the message queue, and this feature was documented from the
beginning in the mq_overview(7) page. In 3.5, some other (useful)
work happened to break the user-space API in a couple of places,
including the value exposed via QSIZE, which now includes a measure
of kernel overhead bytes for the queue, a figure that renders QSIZE
useless for its original purpose, since there's no way to deduce
the number of overhead bytes consumed by the implementation.
(The other user-space breakage was subsequently fixed.)
Hi Marcus,
On 06/13/2015 09:40 PM, Marcus Gelderie wrote:
> A while back, the message queue implementation in the kernel was
> improved to use btrees to speed up retrieval of messages (commit
> d6629859b36). The patch introducing the improved kernel handling of
> message queues has, as a by-product, changed the meaning of the
> QSIZE field in the pseudo-file created for the queue. Before, this
> field reflected the size of the user-data in the queue. Since, it
> now also takes kernel data structures into account, this is no no longer
> true. For example, if 13 bytes of user data are in the queue, on my
> machine the file reports a size of 61 bytes.
Thanks for taking a look at this. This user-space breakage been one of
those nagging things I've wanted to see fixed for a while now, and since
the earlier discussion, I've heard from one or two people who were
unpleasantly surprised by this API change.
> There was some discussion on this topic before (for example
> https://lkml.org/lkml/2014/10/1/115). Reporting the size of the
> message queue in kernel has its merits, but doing so in the QSIZE
> field of the pseudo file corresponding to the queue is a breaking
> change. This patch therefore returns the QSIZE field to its original
> meaning. At the same time, it introduces a new field QKERSIZE that
> reflects the size of the queue in kernel (user data + kernel data).
>
> If the accounting should be improved, I'd be happy to take a look at
> that, too. I would propose to do that in terms of the newly introduced
> field.
The general approach seems good to me. Making the new QKERSIZE the last
field, as you have done, so that any applications that were parsing
an mqueue file in a really dumb fashion (by ordinal position, rather
than checking field names) seems good to me.
Acked-by: Michael Kerrisk <mtk,manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Assuming this approach is acceptable, this patch should eventually go
to stable@ so it is backported to older stable kernels. In the event
that you need to resubmit the patch, I suggest including some of the
background detail I give above in the commit message.
Cheers,
Michael
> Signed-off-by: Marcus Gelderie <redmnic-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> ipc/mqueue.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/ipc/mqueue.c b/ipc/mqueue.c
> index 3aaea7f..7d4c464 100644
> --- a/ipc/mqueue.c
> +++ b/ipc/mqueue.c
> @@ -41,7 +41,7 @@
>
> #define MQUEUE_MAGIC 0x19800202
> #define DIRENT_SIZE 20
> -#define FILENT_SIZE 80
> +#define FILENT_SIZE 90
>
> #define SEND 0
> #define RECV 1
> @@ -82,8 +82,12 @@ struct mqueue_inode_info {
> /* for tasks waiting for free space and messages, respectively */
> struct ext_wait_queue e_wait_q[2];
>
> - unsigned long qsize; /* size of queue in memory (sum of all msgs) */
> -};
> + /* size of queue in memory (sum of all msgs plus kernel
> + * data structures) */
> + unsigned long qsize;
> +
> + /* size of user data in the queue (sum of all msgs) */
> + unsigned long q_usersize; };
>
> static const struct inode_operations mqueue_dir_inode_operations;
> static const struct file_operations mqueue_file_operations;
> @@ -151,6 +155,7 @@ static int msg_insert(struct msg_msg *msg, struct mqueue_inode_info *info)
> insert_msg:
> info->attr.mq_curmsgs++;
> info->qsize += msg->m_ts;
> + info->q_usersize += msg->m_ts;
> list_add_tail(&msg->m_list, &leaf->msg_list);
> return 0;
> }
> @@ -210,6 +215,7 @@ try_again:
> }
> info->attr.mq_curmsgs--;
> info->qsize -= msg->m_ts;
> + info->q_usersize -= msg->m_ts;
> return msg;
> }
>
> @@ -246,6 +252,7 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
> info->notify_owner = NULL;
> info->notify_user_ns = NULL;
> info->qsize = 0;
> + info->q_usersize = 0;
> info->user = NULL; /* set when all is ok */
> info->msg_tree = RB_ROOT;
> info->node_cache = NULL;
> @@ -491,13 +498,14 @@ static ssize_t mqueue_read_file(struct file *filp, char __user *u_data,
>
> spin_lock(&info->lock);
> snprintf(buffer, sizeof(buffer),
> - "QSIZE:%-10lu NOTIFY:%-5d SIGNO:%-5d NOTIFY_PID:%-6d\n",
> - info->qsize,
> + "QSIZE:%-10lu NOTIFY:%-5d SIGNO:%-5d NOTIFY_PID:%-6d QKERSIZE:%-10lu\n",
> + info->q_usersize,
> info->notify_owner ? info->notify.sigev_notify : 0,
> (info->notify_owner &&
> info->notify.sigev_notify == SIGEV_SIGNAL) ?
> info->notify.sigev_signo : 0,
> - pid_vnr(info->notify_owner));
> + pid_vnr(info->notify_owner),
> + info->qsize);
> spin_unlock(&info->lock);
> buffer[sizeof(buffer)-1] = '\0';
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [RFC PATCH 06/18] signal/kthread: Initial implementation of kthread signal handling
From: Petr Mladek @ 2015-06-16 7:54 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Tejun Heo, Ingo Molnar, Peter Zijlstra,
Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150615191429.GA29727-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon 2015-06-15 21:14:29, Oleg Nesterov wrote:
> Ah, understand. You think that we need to take ->siglock in advance
> to avoid the race with SIGCONT?
exactly
> No, we don't. Let me show you the code I suggested again:
>
> void kthread_do_signal_stop(void)
> {
> spin_lock_irq(&curtent->sighand->siglock);
> if (current->jobctl & JOBCTL_STOP_DEQUEUED)
> __set_current_state(TASK_STOPPED);
> spin_unlock_irq(¤t->sighand->siglock);
>
> schedule();
> }
>
> so you can dequeue_signal() and call kthread_do_signal_stop() without
> holding ->siglock. We can rely on JOBCTL_STOP_DEQUEUED bit. SIGCONT
> clears it, so kthread_do_signal_stop() can't race.
Heureka, I have got it. I have previously missed the meaning of the
JOBCTL_STOP_DEQUEUED bit. Thanks for explanation.
Best Regards,
Petr
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox