Re: [PATCH] Documentation: refcount-vs-atomic: Update reference to LKMM doc.

[Andrea Parri]

On Fri, May 04, 2018 at 02:13:59PM -0700, Kees Cook wrote:
> On Fri, May 4, 2018 at 2:11 PM, Andrea Parri
>  wrote:
> > The LKMM project has moved to 'tools/memory-model/'.
> >
> > Signed-off-by: Andrea Parri 
> > ---
> >  Documentation/core-api/refcount-vs-atomic.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/core-api/refcount-vs-atomic.rst 
> > b/Documentation/core-api/refcount-vs-atomic.rst
> > index 83351c258cdb9..322851bada167 100644
> > --- a/Documentation/core-api/refcount-vs-atomic.rst
> > +++ b/Documentation/core-api/refcount-vs-atomic.rst
> > @@ -17,7 +17,7 @@ in order to help maintainers validate their code against 
> > the change in
> >  these memory ordering guarantees.
> >
> >  The terms used through this document try to follow the formal LKMM defined 
> > in
> > -github.com/aparri/memory-model/blob/master/Documentation/explanation.txt
> > +tools/memory-model/Documentation/explanation.txt.
> >
> >  memory-barriers.txt and atomic_t.txt provide more background to the
> >  memory ordering in general and for atomic operations specifically.
> 
> Will this get linkified by rst ?

I believe not, but I'm not too familiar with rst...  FWIW, I'm seeing that
the above memory-barriers.txt, atomic_t.txt are not linkified.

  Andrea


> 
> -Kees
> 
> -- 
> Kees Cook
> Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: refcount-vs-atomic: Update reference to LKMM doc.

[Kees Cook]

On Fri, May 4, 2018 at 2:11 PM, Andrea Parri
 wrote:
> The LKMM project has moved to 'tools/memory-model/'.
>
> Signed-off-by: Andrea Parri 
> ---
>  Documentation/core-api/refcount-vs-atomic.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/core-api/refcount-vs-atomic.rst 
> b/Documentation/core-api/refcount-vs-atomic.rst
> index 83351c258cdb9..322851bada167 100644
> --- a/Documentation/core-api/refcount-vs-atomic.rst
> +++ b/Documentation/core-api/refcount-vs-atomic.rst
> @@ -17,7 +17,7 @@ in order to help maintainers validate their code against 
> the change in
>  these memory ordering guarantees.
>
>  The terms used through this document try to follow the formal LKMM defined in
> -github.com/aparri/memory-model/blob/master/Documentation/explanation.txt
> +tools/memory-model/Documentation/explanation.txt.
>
>  memory-barriers.txt and atomic_t.txt provide more background to the
>  memory ordering in general and for atomic operations specifically.

Will this get linkified by rst ?

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Documentation: refcount-vs-atomic: Update reference to LKMM doc.

[Andrea Parri]

The LKMM project has moved to 'tools/memory-model/'.

Signed-off-by: Andrea Parri 
---
 Documentation/core-api/refcount-vs-atomic.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/core-api/refcount-vs-atomic.rst 
b/Documentation/core-api/refcount-vs-atomic.rst
index 83351c258cdb9..322851bada167 100644
--- a/Documentation/core-api/refcount-vs-atomic.rst
+++ b/Documentation/core-api/refcount-vs-atomic.rst
@@ -17,7 +17,7 @@ in order to help maintainers validate their code against the 
change in
 these memory ordering guarantees.
 
 The terms used through this document try to follow the formal LKMM defined in
-github.com/aparri/memory-model/blob/master/Documentation/explanation.txt
+tools/memory-model/Documentation/explanation.txt.
 
 memory-barriers.txt and atomic_t.txt provide more background to the
 memory ordering in general and for atomic operations specifically.
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/4] seccomp: Audit attempts to modify the actions_logged sysctl

[Steve Grubb]

On Thursday, May 3, 2018 9:08:14 PM EDT Tyler Hicks wrote:
> The decision to log a seccomp action will always be subject to the
> value of the kernel.seccomp.actions_logged sysctl, even for processes
> that are being inspected via the audit subsystem, in an upcoming patch.
> Therefore, we need to emit an audit record on attempts at writing to the
> actions_logged sysctl when auditing is enabled.
> 
> This patch updates the write handler for the actions_logged sysctl to
> emit an audit record on attempts to write to the sysctl. Successful
> writes to the sysctl will result in a record that includes a normalized
> list of logged actions in the "actions" field and a "res" field equal to
> 1. Unsuccessful writes to the sysctl will result in a record that
> doesn't include the "actions" field and has a "res" field equal to 0.
> 
> Not all unsuccessful writes to the sysctl are audited. For example, an
> audit record will not be emitted if an unprivileged process attempts to
> open the sysctl file for reading since that access control check is not
> part of the sysctl's write handler.
> 
> Below are some example audit records when writing various strings to the
> actions_logged sysctl.
> 
> Writing "not-a-real-action", when the kernel.seccomp.actions_logged
> sysctl previously was "kill_process kill_thread trap errno trace log",
> emits this audit record:
> 
>  type=CONFIG_CHANGE msg=audit(1525392371.454:120): op=seccomp-logging
>  actions=? old-actions=kill_process,kill_thread,trap,errno,trace,log
>  res=0
> 
> If you then write "kill_process kill_thread errno trace log", this audit
> record is emitted:
> 
>  type=CONFIG_CHANGE msg=audit(1525392401.645:126): op=seccomp-logging
>  actions=kill_process,kill_thread,errno,trace,log
>  old-actions=kill_process,kill_thread,trap,errno,trace,log res=1
> 
> If you then write "log log errno trace kill_process kill_thread", which
> is unordered and contains the log action twice, it results in the same
> actions value as the previous record:
> 
>  type=CONFIG_CHANGE msg=audit(1525392436.354:132): op=seccomp-logging
>  actions=kill_process,kill_thread,errno,trace,log
>  old-actions=kill_process,kill_thread,errno,trace,log res=1
> 
> If you then write an empty string to the sysctl, this audit record is
> emitted:
> 
>  type=CONFIG_CHANGE msg=audit(1525392494.413:138): op=seccomp-logging
>  actions=(none) old-actions=kill_process,kill_thread,errno,trace,log
>  res=1
> 
> No audit records are generated when reading the actions_logged sysctl.

This appears to cover all the corner cases we talked about. ACK for the 
format of the records.

Thanks,
-Steve
 
> Suggested-by: Steve Grubb 
> Signed-off-by: Tyler Hicks 
> ---
>  include/linux/audit.h |  5 +
>  kernel/auditsc.c  | 20 ++
>  kernel/seccomp.c  | 58
> +++ 3 files changed, 74
> insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 75d5b03..d4e35e7 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -233,6 +233,8 @@ extern void __audit_inode_child(struct inode *parent,
>   const struct dentry *dentry,
>   const unsigned char type);
>  extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> +extern void audit_seccomp_actions_logged(const char *names,
> +  const char *old_names, int res);
>  extern void __audit_ptrace(struct task_struct *t);
> 
>  static inline bool audit_dummy_context(void)
> @@ -502,6 +504,9 @@ static inline void __audit_seccomp(unsigned long
> syscall, long signr, int code) { }
>  static inline void audit_seccomp(unsigned long syscall, long signr, int
> code) { }
> +static inline void audit_seccomp_actions_logged(const char *names,
> + const char *old_names, int res)
> +{ }
>  static inline int auditsc_get_stamp(struct audit_context *ctx,
> struct timespec64 *t, unsigned int *serial)
>  {
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4e0a4ac..5195a29 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2478,6 +2478,26 @@ void __audit_seccomp(unsigned long syscall, long
> signr, int code) audit_log_end(ab);
>  }
> 
> +void audit_seccomp_actions_logged(const char *names, const char
> *old_names, +   int res)
> +{
> + struct audit_buffer *ab;
> +
> + if (!audit_enabled)
> + return;
> +
> + ab = audit_log_start(current->audit_context, GFP_KERNEL,
> +  AUDIT_CONFIG_CHANGE);
> + if (unlikely(!ab))
> + return;
> +
> + audit_log_format(ab, "op=seccomp-logging");
> + audit_log_format(ab, " actions=%s", names);
> + audit_log_format(ab, " old-actions=%s", old_names);
> + audit_log_format(ab, " res=%d", res);
> + 

Re: [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

[Ganapatrao Kulkarni]

Hi Mark,

On Thu, Apr 26, 2018 at 4:29 PM, Mark Rutland  wrote:
> Hi,
>
> On Wed, Apr 25, 2018 at 02:30:47PM +0530, Ganapatrao Kulkarni wrote:
>> +
>> +/* L3c and DMC has 16 and 8 channels per socket respectively.
>> + * Each Channel supports UNCORE PMU device and consists of
>> + * 4 independent programmable counters. Counters are 32 bit
>> + * and does not support overflow interrupt, they needs to be
>> + * sampled before overflow(i.e, at every 2 seconds).
>> + */
>> +
>> +#define UNCORE_MAX_COUNTERS  4
>> +#define UNCORE_L3_MAX_TILES  16
>> +#define UNCORE_DMC_MAX_CHANNELS  8
>> +
>> +#define UNCORE_HRTIMER_INTERVAL  (2 * NSEC_PER_SEC)
>
> How was a period of two seconds chosen?

It has been suggested from hw team  to sample before  3 to 4 seconds.

>
> What's the maximum clock speed for the L3C and DMC?

L3C at 1.5GHz and DMC at 1.2GHz
>
> Given that all channels compete for access to the muxed register
> interface, I suspect we need to try more often than once every 2
> seconds...

2 seconds seems to be sufficient. So far testing looks good.

>
> [...]
>
>> +struct active_timer {
>> + struct perf_event *event;
>> + struct hrtimer hrtimer;
>> +};
>> +
>> +/*
>> + * pmu on each socket has 2 uncore devices(dmc and l3),
>> + * each uncore device has up to 16 channels, each channel can sample
>> + * events independently with counters up to 4.
>> + *
>> + * struct thunderx2_pmu_uncore_channel created per channel.
>> + * struct thunderx2_pmu_uncore_dev per uncore device.
>> + */
>> +struct thunderx2_pmu_uncore_channel {
>> + struct thunderx2_pmu_uncore_dev *uncore_dev;
>> + struct pmu pmu;
>
> Can we put the pmu first in the struct, please?

ok
>
>> + int counter;
>
> AFAICT, this counter field is never used.

hmm ok, will remove.
>
>> + int channel;
>> + DECLARE_BITMAP(counter_mask, UNCORE_MAX_COUNTERS);
>> + struct active_timer *active_timers;
>
> You should only need a single timer per channel, rather than one per
> event.
>
> I think you can get rid of the active_timer structure, and have:
>
> struct perf_event *events[UNCORE_MAX_COUNTERS];
> struct hrtimer timer;
>

thanks, will change as suggested.

>> + /* to sync counter alloc/release */
>> + raw_spinlock_t lock;
>> +};
>> +
>> +struct thunderx2_pmu_uncore_dev {
>> + char *name;
>> + struct device *dev;
>> + enum thunderx2_uncore_type type;
>> + unsigned long base;
>
> This should be:
>
> void __iomem *base;

ok
>
> [...]
>
>> +static ssize_t cpumask_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct cpumask cpu_mask;
>> + struct thunderx2_pmu_uncore_channel *pmu_uncore =
>> + pmu_to_thunderx2_pmu_uncore(dev_get_drvdata(dev));
>> +
>> + /* Pick first online cpu from the node */
>> + cpumask_clear(_mask);
>> + cpumask_set_cpu(cpumask_first(
>> + cpumask_of_node(pmu_uncore->uncore_dev->node)),
>> + _mask);
>> +
>> + return cpumap_print_to_pagebuf(true, buf, _mask);
>> +}
>
> AFAICT cpumask_of_node() returns a mask that can include offline CPUs.
>
> Regardless, I don't think that you can keep track of the management CPU
> this way. Please keep track of the CPU the PMU should be managed by,
> either with a cpumask or int embedded within the PMU structure.

thanks, will add hotplug callbacks.
>
> At hotplug time, you'll need to update the management CPU, calling
> perf_pmu_migrate_context() when it is offlined.

ok
>
> [...]
>
>> +static int alloc_counter(struct thunderx2_pmu_uncore_channel *pmu_uncore)
>> +{
>> + int counter;
>> +
>> + raw_spin_lock(_uncore->lock);
>> + counter = find_first_zero_bit(pmu_uncore->counter_mask,
>> + pmu_uncore->uncore_dev->max_counters);
>> + if (counter == pmu_uncore->uncore_dev->max_counters) {
>> + raw_spin_unlock(_uncore->lock);
>> + return -ENOSPC;
>> + }
>> + set_bit(counter, pmu_uncore->counter_mask);
>> + raw_spin_unlock(_uncore->lock);
>> + return counter;
>> +}
>> +
>> +static void free_counter(struct thunderx2_pmu_uncore_channel *pmu_uncore,
>> + int counter)
>> +{
>> + raw_spin_lock(_uncore->lock);
>> + clear_bit(counter, pmu_uncore->counter_mask);
>> + raw_spin_unlock(_uncore->lock);
>> +}
>
> I don't believe that locking is required in either of these, as the perf
> core serializes pmu::add() and pmu::del(), where these get called.

thanks, it seems to be not required.
>
>> +
>> +/*
>> + * DMC and L3 counter interface is muxed across all channels.
>> + * hence we need to select the channel before accessing counter
>> + * data/control registers.
>
> Are there separate interfaces for all-dmc-channels and all-l3c-channels?

separate interface for DMC and L3c.
channels within DMC/L3C are muxed.

>
> ... or is a 

Re: [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

[Robin Murphy]

Hi Kim,

On 04/05/18 01:30, Kim Phillips wrote:

On Tue, 1 May 2018 12:54:05 +0100
Will Deacon  wrote:


Hi Kim,


Hi Will, thanks for responding.


On Fri, Apr 27, 2018 at 11:56:25AM -0500, Kim Phillips wrote:

On Fri, 27 Apr 2018 17:09:14 +0100
Will Deacon  wrote:

On Fri, Apr 27, 2018 at 10:46:29AM -0500, Kim Phillips wrote:

On Fri, 27 Apr 2018 15:37:20 +0100
Will Deacon  wrote:

For anything under drivers/perf/, I'd prefer not to have these prints
and instead see efforts to improve error reporting via the perf system
call interface.


We'd all prefer that, and for all PMU drivers, why should ones under
drivers/perf be treated differently?


Because they're the ones I maintain...


You represent a minority on your opinion on this matter though.


I'm not sure that's true


I haven't seen another arch/PMU driver maintainer actively oppose it
other than you (and Mark).


-- you tend to be the only one raising this issue;


If you're basing that on list activity, I wouldn't, since most Arm perf
users don't subscribe to LKML.  I'm trying to represent current and
future Arm perf users that shouldn't be expected to hang out here on
LKML and review PMU drivers.  Couldn't tell you why PMU driver authors
themselves don't chime in, but will note that some drivers in
drivers/perf do print things from their event_init functions.


As a PMU driver author who *has* already invested not-inconsiderable 
time and effort in trying to explain to you the technical issues from a 
non-maintainer perspective (and is about to do so again), I can't help 
but feel rather slighted by that comment.



Also, you have been cc'd on off-list email threads where SPE users had
to debug the SPE driver's event_init() manually.  There are other SPE
users I know that have had to go through the same painstaking process of
debugging the SPE driver.  Last but not least, there's me, and I'd sure
appreciate more verbose PMU drivers, based on my real-life performance
monitoring experience(s).


I think most people don't actually care one way or the other. If you know of


Like I said, I think you're limiting your audience to LKML subscribers,
who are more amenable to being convinced they need to debug their
kernel.


others who care about it too then I suggest you work together as a group to
solve the problem in a way which is amenable to the upstream maintainers.


I know a few off-list people that would like a more user-friendly Arm
perf experience, but we're not qualified to solve the larger problem
that perf has had since its inception, and I think it's a little unfair
for you to suggest that, esp. given you're one of the maintainers, and
there's Linus on top of you.


Then you won't have to worry about fixing it personally. You could even
propose a topic for plumbers if there is enough interest in a general
framework for propagating error messages to userspace.


I'm not worried about fixing it personally or not.  I'm worried about
our perf users' experience given your objection to using the vehicle
already in use on other architectures and PMU drivers.

If you - or anyone else for that matter - know of a way that'll get us
past this, by all means, say something.

If it helps, I recently asked acme if we could borrow bits from struct
perf_event and got no response.


As you are already aware, I've personally tried to fix this problem -
that has existed since before the introduction of the perf tool (I
consider it a syscall-independent enhanced error interface), multiple
times, and failed.


Why is that my problem? Try harder?


It's your problem because we're here reviewing a patch that happens to
fall under your maintainership.  I'll be the first person to tell you
I'm obviously incompetent and haven't been able to come up with a
solution that is acceptable for everyone up to and including Linus
Torvalds.  I'm just noticing a chronic usability problem that can be
easily alleviated in the context of this patch review.


I don't see how incompetence has anything to do with it and, like most


Well you told me to try harder, and I'm trying to let you know that I
can try harder - ad infinitum - and still not get it, so telling me to
try harder isn't going to necessarily resolve the issue.


people (lucky gits...), I doubt Torvalds cares too deeply about PMU driver
internals. No arguments on the usability problem, but this ain't the fix


Like acme, I doubt Linus runs perf on real Arm h/w, so yes, I don't
think they care that much, but if they did try to do it one day, I'd
like to that they'd be able to have dmesg contain that extra error
information for them.

As you know, David Howells wrote a supplemental error syscall facility
[1], that Linus nacked (off list no less).  Does Linus care about what
David Howells was developing it for?  I don't know, but he does have
enough experience to know whether a certain approach to a problem is
sustainable in his kernel or not.


you're 

[PATCH] libata: remove ata_sff_data_xfer_noirq()

[Sebastian Andrzej Siewior]

ata_sff_data_xfer_noirq() is invoked via the ->sff_data_xfer hook. The
latter is invoked by ata_pio_sector(), atapi_send_cdb() and
__atapi_pio_bytes() which in turn is invoked by ata_sff_hsm_move().
The latter function requires that the "ap->lock" lock is held which
needs to be taken with disabled interrupts.

There is no need have to have ata_sff_data_xfer_noirq() which invokes
ata_sff_data_xfer32() with disabled interrupts because at this point the
interrupts are already disabled.
Remove the function and its references to it and replace all callers
with ata_sff_data_xfer32().

Signed-off-by: Sebastian Andrzej Siewior 
---
 Documentation/driver-api/libata.rst |  3 +--
 drivers/ata/libata-sff.c| 30 -
 drivers/ata/pata_cmd640.c   |  2 +-
 drivers/ata/pata_icside.c   |  2 +-
 drivers/ata/pata_imx.c  |  2 +-
 drivers/ata/pata_legacy.c   |  6 +++---
 drivers/ata/pata_palmld.c   |  2 +-
 drivers/ata/pata_pcmcia.c   |  2 +-
 drivers/ata/pata_platform.c |  2 +-
 drivers/ata/pata_via.c  |  2 +-
 include/linux/libata.h  |  2 --
 11 files changed, 11 insertions(+), 44 deletions(-)

diff --git a/Documentation/driver-api/libata.rst 
b/Documentation/driver-api/libata.rst
index 4adc056f7635..70e180e6b93d 100644
--- a/Documentation/driver-api/libata.rst
+++ b/Documentation/driver-api/libata.rst
@@ -118,8 +118,7 @@ PIO data read/write
 All bmdma-style drivers must implement this hook. This is the low-level
 operation that actually copies the data bytes during a PIO data
 transfer. Typically the driver will choose one of
-:c:func:`ata_sff_data_xfer_noirq`, :c:func:`ata_sff_data_xfer`, or
-:c:func:`ata_sff_data_xfer32`.
+:c:func:`ata_sff_data_xfer`, or :c:func:`ata_sff_data_xfer32`.
 
 ATA command execute
 ~~~
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index cc2f2e35f4c2..c5ea0fc635e5 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -657,36 +657,6 @@ unsigned int ata_sff_data_xfer32(struct ata_queued_cmd 
*qc, unsigned char *buf,
 }
 EXPORT_SYMBOL_GPL(ata_sff_data_xfer32);
 
-/**
- * ata_sff_data_xfer_noirq - Transfer data by PIO
- * @qc: queued command
- * @buf: data buffer
- * @buflen: buffer length
- * @rw: read/write
- *
- * Transfer data from/to the device data register by PIO. Do the
- * transfer with interrupts disabled.
- *
- * LOCKING:
- * Inherited from caller.
- *
- * RETURNS:
- * Bytes consumed.
- */
-unsigned int ata_sff_data_xfer_noirq(struct ata_queued_cmd *qc, unsigned char 
*buf,
-unsigned int buflen, int rw)
-{
-   unsigned long flags;
-   unsigned int consumed;
-
-   local_irq_save(flags);
-   consumed = ata_sff_data_xfer32(qc, buf, buflen, rw);
-   local_irq_restore(flags);
-
-   return consumed;
-}
-EXPORT_SYMBOL_GPL(ata_sff_data_xfer_noirq);
-
 /**
  * ata_pio_sector - Transfer a sector of data.
  * @qc: Command on going
diff --git a/drivers/ata/pata_cmd640.c b/drivers/ata/pata_cmd640.c
index c47caa807fa9..e3532eda7b05 100644
--- a/drivers/ata/pata_cmd640.c
+++ b/drivers/ata/pata_cmd640.c
@@ -178,7 +178,7 @@ static struct scsi_host_template cmd640_sht = {
 static struct ata_port_operations cmd640_port_ops = {
.inherits   = _sff_port_ops,
/* In theory xfer_noirq is not needed once we kill the prefetcher */
-   .sff_data_xfer  = ata_sff_data_xfer_noirq,
+   .sff_data_xfer  = ata_sff_data_xfer32,
.sff_irq_check  = cmd640_sff_irq_check,
.qc_issue   = cmd640_qc_issue,
.cable_detect   = ata_cable_40wire,
diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index 188f2f2eb21f..c272f2cbb47c 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -324,7 +324,7 @@ static struct ata_port_operations pata_icside_port_ops = {
.inherits   = _bmdma_port_ops,
/* no need to build any PRD tables for DMA */
.qc_prep= ata_noop_qc_prep,
-   .sff_data_xfer  = ata_sff_data_xfer_noirq,
+   .sff_data_xfer  = ata_sff_data_xfer32,
.bmdma_setup= pata_icside_bmdma_setup,
.bmdma_start= pata_icside_bmdma_start,
.bmdma_stop = pata_icside_bmdma_stop,
diff --git a/drivers/ata/pata_imx.c b/drivers/ata/pata_imx.c
index d4caa23f5a88..108101325efd 100644
--- a/drivers/ata/pata_imx.c
+++ b/drivers/ata/pata_imx.c
@@ -102,7 +102,7 @@ static struct scsi_host_template pata_imx_sht = {
 
 static struct ata_port_operations pata_imx_port_ops = {
.inherits   = _sff_port_ops,
-   .sff_data_xfer  = ata_sff_data_xfer_noirq,
+   .sff_data_xfer  = ata_sff_data_xfer32,
.cable_detect   = ata_cable_unknown,
.set_piomode= pata_imx_set_piomode,
 

Re: [PATCH v3 0/9] trace_uprobe: Support SDT markers having reference count (semaphore)

[Oleg Nesterov]

Sorry Ravi, I saved the new version for review and forgot about it... I'll
try to do this on weekend.

On 05/03, Ravi Bangoria wrote:
> 
> On 04/17/2018 10:02 AM, Ravi Bangoria wrote:
> > Userspace Statically Defined Tracepoints[1] are dtrace style markers
> > inside userspace applications. Applications like PostgreSQL, MySQL,
> > Pthread, Perl, Python, Java, Ruby, Node.js, libvirt, QEMU, glib etc
> > have these markers embedded in them. These markers are added by developer
> > at important places in the code. Each marker source expands to a single
> > nop instruction in the compiled code but there may be additional
> > overhead for computing the marker arguments which expands to couple of
> > instructions. In case the overhead is more, execution of it can be
> > omitted by runtime if() condition when no one is tracing on the marker:
> >
> > if (reference_counter > 0) {
> > Execute marker instructions;
> > }   
> >
> > Default value of reference counter is 0. Tracer has to increment the 
> > reference counter before tracing on a marker and decrement it when
> > done with the tracing.
> 
> Hi Oleg, Masami,
> 
> Can you please review this :) ?
> 
> Thanks.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 6/9] trace_uprobe: Support SDT markers having reference count (semaphore)

[Ravi Bangoria]

Hi Masami,

On 05/04/2018 10:18 AM, Masami Hiramatsu wrote:
>> +void uprobe_down_write_dup_mmap(void)
>> +{
>> +percpu_down_write(_mmap_sem);
>> +}
>> +
>> +void uprobe_up_write_dup_mmap(void)
>> +{
>> +percpu_up_write(_mmap_sem);
>> +}
>> +
> I'm not sure why these hunks are not done in previous patch.
> If you separate "uprobe_map_info" export patch, this also
> should be separated. (Or both merged into this patch)

Sure, I'll add separate patch for dup_mmap_sem.

>> +/*
>> + * Reference counter gate the invocation of probe. If present,
>> + * by default reference counter is 0. One needs to increment
>> + * it before tracing the probe and decrement it when done.
>> + */
>> +static int
>> +sdt_update_ref_ctr(struct mm_struct *mm, unsigned long vaddr, short d)
>> +{
>> +void *kaddr;
>> +struct page *page;
>> +struct vm_area_struct *vma;
>> +int ret = 0;
>> +unsigned short *ptr;
>> +
>> +if (vaddr == 0)
>> +return -EINVAL;
>> +
>> +ret = get_user_pages_remote(NULL, mm, vaddr, 1,
>> +FOLL_FORCE | FOLL_WRITE, , , NULL);
>> +if (ret <= 0)
>> +return ret;
> Hmm, get_user_pages_remote() said
>
> ===
> If nr_pages is 0 or negative, returns 0. If no pages were pinned, returns 
> -errno.
> ===
>
> And you've passed 1 for nr_pages, so it must be 1 or -errno.
>
>> +
>> +kaddr = kmap_atomic(page);
>> +ptr = kaddr + (vaddr & ~PAGE_MASK);
>> +*ptr += d;
>> +kunmap_atomic(kaddr);
>> +
>> +put_page(page);
>> +return 0;
> And obviously 0 means "success" for sdt_update_ref_ctr().
> I think if get_user_pages_remote returns 0, this should
> return -EBUSY (*) or something else.
>
> * It seems that if faultin_page() in __get_user_pages()
> returns -EBUSY, get_user_pages_remote() can return 0.

Ah good catch :). Will change it.

>> +}
>> +
>> +static void sdt_increment_ref_ctr(struct trace_uprobe *tu)
>> +{
>> +struct uprobe_map_info *info;
>> +
>> +uprobe_down_write_dup_mmap();
>> +info = uprobe_build_map_info(tu->inode->i_mapping,
>> +tu->ref_ctr_offset, false);
>> +if (IS_ERR(info))
>> +goto out;
>> +
>> +while (info) {
>> +down_write(>mm->mmap_sem);
>> +
>> +if (sdt_find_vma(tu, info->mm, info->vaddr))
>> +sdt_update_ref_ctr(info->mm, info->vaddr, 1);
> Don't you have to handle the error to map pages here?

Correct.. I think, I've to feedback error code to probe_event_{enable|disable}
and handler failure there.

Thanks for the review,
Ravi

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MAINTAINERS & files: Canonize the e-mails I use at files

[Mauro Carvalho Chehab]

Em Fri, 04 May 2018 13:58:39 +0300
Jani Nikula  escreveu:

> On Fri, 04 May 2018, Mauro Carvalho Chehab  wrote:
> > From now on, I'll start using my @kernel.org as my development e-mail.
> >
> > As such, let's remove the entries that point to the old
> > mche...@s-opensource.com at MAINTAINERS file.
> >
> > For the files written with a copyright with mchehab@s-opensource,
> > let's keep Samsung on their names, using mchehab+sams...@kernel.org,
> > in order to keep pointing to my employer, with sponsors the work.
> >
> > For the files written before I join Samsung (on July, 4 2013),
> > let's just use mche...@kernel.org.
> >
> > For bug reports, we can simply point to just kernel.org, as
> > this will reach my mchehab+samsung inbox anyway.  
> 
> I suppose this begs the question, why do we insist on adding our email
> addresses all over the place? On a quick grep, there are at least 40k+
> email addresses in the sources. Do we expect them all to be up-to-date
> too?

That's a good question.

The usual use case is that the e-mail allows people to contact developers
if needed. Such contact could simply due to something like handling SPDX
or other license-related issues or for troubleshooting.

There's also another reason (with IMHO, is more relevant): just the name
may not be enough to uniquely identify the author of some code. While
that might happen on occidental Countries, this is a way more relevant
for Asian Countries. For example, there are very few surnames on
some Countries there[1], and common names are usually... common. So, it
is not hard to find several people with exactly the same name working at
the same company. I've seen e-mails from those people that are things like
john.doe51@some.company, john.doe69@some.company, ...

[1] For example: https://en.wikipedia.org/wiki/List_of_Korean_surnames.

The e-mail is a way to uniquely identify a person. If we remove it,
then we may need to add another thing instead (like parents names,
security number or whatever), with would be weird, IMO. 

As we all use e-mails to uniquely identify contributors submissions,
IMHO, the best is to keep using e-mails. The side effect is that
we should keep those emails updated.

-

In the specific case of this patch, as I'm now just using @kernel.org
everywhere within the Kernel tree, I don't expect needing to change
it in long term.

Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MAINTAINERS & files: Canonize the e-mails I use at files

[Jani Nikula]

On Fri, 04 May 2018, Mauro Carvalho Chehab  wrote:
> From now on, I'll start using my @kernel.org as my development e-mail.
>
> As such, let's remove the entries that point to the old
> mche...@s-opensource.com at MAINTAINERS file.
>
> For the files written with a copyright with mchehab@s-opensource,
> let's keep Samsung on their names, using mchehab+sams...@kernel.org,
> in order to keep pointing to my employer, with sponsors the work.
>
> For the files written before I join Samsung (on July, 4 2013),
> let's just use mche...@kernel.org.
>
> For bug reports, we can simply point to just kernel.org, as
> this will reach my mchehab+samsung inbox anyway.

I suppose this begs the question, why do we insist on adding our email
addresses all over the place? On a quick grep, there are at least 40k+
email addresses in the sources. Do we expect them all to be up-to-date
too?


BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] MAINTAINERS & files: Canonize the e-mails I use at files

[Mauro Carvalho Chehab]

>From now on, I'll start using my @kernel.org as my development e-mail.

As such, let's remove the entries that point to the old
mche...@s-opensource.com at MAINTAINERS file.

For the files written with a copyright with mchehab@s-opensource,
let's keep Samsung on their names, using mchehab+sams...@kernel.org,
in order to keep pointing to my employer, with sponsors the work.

For the files written before I join Samsung (on July, 4 2013),
let's just use mche...@kernel.org.

For bug reports, we can simply point to just kernel.org, as
this will reach my mchehab+samsung inbox anyway.

Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Brian Warner 
Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/doc-guide/parse-headers.rst   |  4 ++--
 Documentation/media/uapi/rc/keytable.c.rst  |  2 +-
 Documentation/media/uapi/v4l/v4l2grab.c.rst |  2 +-
 Documentation/sphinx/parse-headers.pl   |  4 ++--
 .../zh_CN/video4linux/v4l2-framework.txt|  4 ++--
 MAINTAINERS | 17 -
 drivers/media/i2c/saa7115.c |  2 +-
 drivers/media/i2c/saa711x_regs.h|  2 +-
 drivers/media/i2c/tda7432.c |  2 +-
 drivers/media/i2c/tvp5150.c |  2 +-
 drivers/media/i2c/tvp5150_reg.h |  2 +-
 drivers/media/i2c/tvp7002.c |  2 +-
 drivers/media/i2c/tvp7002_reg.h |  2 +-
 drivers/media/media-devnode.c   |  2 +-
 drivers/media/pci/bt8xx/bttv-audio-hook.c   |  2 +-
 drivers/media/pci/bt8xx/bttv-audio-hook.h   |  2 +-
 drivers/media/pci/bt8xx/bttv-cards.c|  4 ++--
 drivers/media/pci/bt8xx/bttv-driver.c   |  2 +-
 drivers/media/pci/bt8xx/bttv-i2c.c  |  2 +-
 drivers/media/pci/cx23885/cx23885-input.c   |  2 +-
 drivers/media/pci/cx88/cx88-alsa.c  |  4 ++--
 drivers/media/pci/cx88/cx88-blackbird.c |  2 +-
 drivers/media/pci/cx88/cx88-core.c  |  2 +-
 drivers/media/pci/cx88/cx88-i2c.c   |  2 +-
 drivers/media/pci/cx88/cx88-video.c |  2 +-
 drivers/media/radio/radio-aimslab.c |  2 +-
 drivers/media/radio/radio-aztech.c  |  2 +-
 drivers/media/radio/radio-gemtek.c  |  2 +-
 drivers/media/radio/radio-maxiradio.c   |  2 +-
 drivers/media/radio/radio-rtrack2.c |  2 +-
 drivers/media/radio/radio-sf16fmi.c |  2 +-
 drivers/media/radio/radio-terratec.c|  2 +-
 drivers/media/radio/radio-trust.c   |  2 +-
 drivers/media/radio/radio-typhoon.c |  2 +-
 drivers/media/radio/radio-zoltrix.c |  2 +-
 drivers/media/rc/keymaps/rc-avermedia-m135a.c   |  2 +-
 drivers/media/rc/keymaps/rc-encore-enltv-fm53.c |  2 +-
 drivers/media/rc/keymaps/rc-encore-enltv2.c |  2 +-
 drivers/media/rc/keymaps/rc-kaiomy.c|  2 +-
 .../media/rc/keymaps/rc-kworld-plus-tv-analog.c |  2 +-
 drivers/media/rc/keymaps/rc-pixelview-new.c |  2 +-
 drivers/media/tuners/tea5761.c  |  4 ++--
 drivers/media/tuners/tea5767.c  |  4 ++--
 drivers/media/tuners/tuner-xc2028-types.h   |  2 +-
 drivers/media/tuners/tuner-xc2028.c |  4 ++--
 drivers/media/tuners/tuner-xc2028.h |  2 +-
 drivers/media/usb/em28xx/em28xx-camera.c|  2 +-
 drivers/media/usb/em28xx/em28xx-cards.c |  2 +-
 drivers/media/usb/em28xx/em28xx-core.c  |  4 ++--
 drivers/media/usb/em28xx/em28xx-dvb.c   |  4 ++--
 drivers/media/usb/em28xx/em28xx-i2c.c   |  2 +-
 drivers/media/usb/em28xx/em28xx-input.c |  2 +-
 drivers/media/usb/em28xx/em28xx-video.c |  4 ++--
 drivers/media/usb/em28xx/em28xx.h   |  2 +-
 drivers/media/usb/gspca/zc3xx-reg.h |  2 +-
 drivers/media/usb/tm6000/tm6000-cards.c |  2 +-
 drivers/media/usb/tm6000/tm6000-core.c  |  2 +-
 drivers/media/usb/tm6000/tm6000-i2c.c   |  2 +-
 drivers/media/usb/tm6000/tm6000-regs.h  |  2 +-
 drivers/media/usb/tm6000/tm6000-usb-isoc.h  |  2 +-
 drivers/media/usb/tm6000/tm6000-video.c |  2 +-
 drivers/media/usb/tm6000/tm6000.h   |  2 +-
 drivers/media/v4l2-core/v4l2-dev.c  |  4 ++--
 drivers/media/v4l2-core/v4l2-ioctl.c|  2 +-
 drivers/media/v4l2-core/videobuf-core.c |  6 +++---
 drivers/media/v4l2-core/videobuf-dma-contig.c   |  2 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c   |  6 +++---
 drivers/media/v4l2-core/videobuf-vmalloc.c  |  4 ++--
 include/media/i2c/tvp7002.h |  2 +-
 include/media/videobuf-core.h   |  4 ++--
 include/media/videobuf-dma-sg.h |  4 ++--
 include/media/videobuf-vmalloc.h|  2 +-
 scripts/extract_xc3028.pl   |  2 +-