Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Russell King - ARM Linux]

On Sat, Mar 11, 2017 at 07:31:18PM -0800, Steve Longerbeam wrote:
> 
> 
> On 03/11/2017 10:59 AM, Russell King - ARM Linux wrote:
> >On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:
> >>
> >>
> >>On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:
> >>>I really don't think expecting the user to understand and configure
> >>>the pipeline is a sane way forward.  Think about it - should the
> >>>user need to know that, because they have a bayer-only CSI data
> >>>source, that there is only one path possible, and if they try to
> >>>configure a different path, then things will just error out?
> >>>
> >>>For the case of imx219 connected to iMX6, it really is as simple as
> >>>"there is only one possible path" and all the complexity of the media
> >>>interfaces/subdevs is completely unnecessary.  Every other block in
> >>>the graph is just noise.
> >>>
> >>>The fact is that these dot graphs show a complex picture, but reality
> >>>is somewhat different - there's only relatively few paths available
> >>>depending on the connected source and the rest of the paths are
> >>>completely useless.
> >>>
> >>
> >>I totally disagree there. Raw bayer requires passthrough yes, but for
> >>all other media bus formats on a mipi csi-2 bus, and all other media
> >>bus formats on 8-bit parallel buses, the conersion pipelines can be
> >>used for scaling, CSC, rotation, and motion-compensated de-interlacing.
> >
> >... which only makes sense _if_ your source can produce those formats.
> >We don't actually disagree on that.
> 
> ...and there are lots of those sources! You should try getting out of
> your imx219 shell some time, and have a look around! :)

If you think that, you are insulting me.  I've been thinking about this
from the "big picture" point of view.  If you think I'm only thinking
about this from only the bayer point of view, you're wrong.

Given what Mauro has said, I'm convinced that the media controller stuff
is a complete failure for usability, and adding further drivers using it
is a mistake.

I counter your accusation by saying that you are actually so focused on
the media controller way of doing things that you can't see the bigger
picture here.

So, tell me how the user can possibly use iMX6 video capture without
resorting to opening up a terminal and using media-ctl to manually
configure the pipeline.  How is the user going to control the source
device without using media-ctl to find the subdev node, and then using
v4l2-ctl on it.  How is the user supposed to know which /dev/video*
node they should be opening with their capture application?

If you can actually respond to the points that I've been raising about
end user usability, then we can have a discussion.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Russell King - ARM Linux]

On Sat, Mar 11, 2017 at 07:31:18PM -0800, Steve Longerbeam wrote:
> 
> 
> On 03/11/2017 10:59 AM, Russell King - ARM Linux wrote:
> >On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:
> >>
> >>
> >>On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:
> >>>I really don't think expecting the user to understand and configure
> >>>the pipeline is a sane way forward.  Think about it - should the
> >>>user need to know that, because they have a bayer-only CSI data
> >>>source, that there is only one path possible, and if they try to
> >>>configure a different path, then things will just error out?
> >>>
> >>>For the case of imx219 connected to iMX6, it really is as simple as
> >>>"there is only one possible path" and all the complexity of the media
> >>>interfaces/subdevs is completely unnecessary.  Every other block in
> >>>the graph is just noise.
> >>>
> >>>The fact is that these dot graphs show a complex picture, but reality
> >>>is somewhat different - there's only relatively few paths available
> >>>depending on the connected source and the rest of the paths are
> >>>completely useless.
> >>>
> >>
> >>I totally disagree there. Raw bayer requires passthrough yes, but for
> >>all other media bus formats on a mipi csi-2 bus, and all other media
> >>bus formats on 8-bit parallel buses, the conersion pipelines can be
> >>used for scaling, CSC, rotation, and motion-compensated de-interlacing.
> >
> >... which only makes sense _if_ your source can produce those formats.
> >We don't actually disagree on that.
> 
> ...and there are lots of those sources! You should try getting out of
> your imx219 shell some time, and have a look around! :)

If you think that, you are insulting me.  I've been thinking about this
from the "big picture" point of view.  If you think I'm only thinking
about this from only the bayer point of view, you're wrong.

Given what Mauro has said, I'm convinced that the media controller stuff
is a complete failure for usability, and adding further drivers using it
is a mistake.

I counter your accusation by saying that you are actually so focused on
the media controller way of doing things that you can't see the bigger
picture here.

So, tell me how the user can possibly use iMX6 video capture without
resorting to opening up a terminal and using media-ctl to manually
configure the pipeline.  How is the user going to control the source
device without using media-ctl to find the subdev node, and then using
v4l2-ctl on it.  How is the user supposed to know which /dev/video*
node they should be opening with their capture application?

If you can actually respond to the points that I've been raising about
end user usability, then we can have a discussion.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2] MAINTAINERS: Add maintianer entry for crypto/s5p-sss

[Vladimir Zapolskiy]

On 03/11/2017 08:11 AM, Krzysztof Kozlowski wrote:
> Add Krzysztof Kozlowski and Vladimir Zapolskiy as maintainers of s5p-sss
> driver for handling reviews, testing and getting bug reports from the
> users.
> 
> Cc: Vladimir Zapolskiy 
> Cc: Herbert Xu 
> Signed-off-by: Krzysztof Kozlowski 
> 

Acked-by: Vladimir Zapolskiy 

--
With best wishes,
Vladimir

Re: [PATCH v2] MAINTAINERS: Add maintianer entry for crypto/s5p-sss

[Vladimir Zapolskiy]

On 03/11/2017 08:11 AM, Krzysztof Kozlowski wrote:
> Add Krzysztof Kozlowski and Vladimir Zapolskiy as maintainers of s5p-sss
> driver for handling reviews, testing and getting bug reports from the
> users.
> 
> Cc: Vladimir Zapolskiy 
> Cc: Herbert Xu 
> Signed-off-by: Krzysztof Kozlowski 
> 

Acked-by: Vladimir Zapolskiy 

--
With best wishes,
Vladimir

[PATCH v3] statx: optimize copy of struct statx to userspace

[Eric Biggers]

From: Eric Biggers 

I found that statx() was significantly slower than stat().  As a
microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
file to the same with statx() passed a NULL path:

$ time ./stat_benchmark

real0m1.464s
user0m0.275s
sys 0m1.187s

$ time ./statx_benchmark

real0m5.530s
user0m0.281s
sys 0m5.247s

statx is expected to be a little slower than stat because struct statx
is larger than struct stat, but not by *that* much.  It turns out that
most of the overhead was in copying struct statx to userspace, mostly in
all the stac/clac instructions that got generated for each __put_user()
call.  (This was on x86_64, but some other architectures, e.g. arm64,
have something similar now too.)

stat() instead initializes its struct on the stack and copies it to
userspace with a single call to copy_to_user().  This turns out to be
much faster, and changing statx to do this makes it almost as fast as
stat:

$ time ./statx_benchmark

real0m1.624s
user0m0.270s
sys 0m1.354s

Implementation-wise, for zeroing the reserved fields, I chose to simply
start by zeroing the full struct with memset.  This makes it clear that
every byte copied to userspace is initialized, even implicit padding
bytes (though there are none currently).  In the scenarios I tested, it
also performed the same as a designated initializer.  Manually
initializing each field was still slightly faster, but would have been
more error-prone and less verifiable.

Signed-off-by: Eric Biggers 
---
 fs/stat.c | 73 +++
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/fs/stat.c b/fs/stat.c
index fa0be59340cc..3fbecbfa6975 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -509,46 +509,36 @@ SYSCALL_DEFINE4(fstatat64, int, dfd, const char __user *, 
filename,
 }
 #endif /* __ARCH_WANT_STAT64 || __ARCH_WANT_COMPAT_STAT64 */
 
-static inline int __put_timestamp(struct timespec *kts,
- struct statx_timestamp __user *uts)
+static int cp_statx(const struct kstat *stat, struct statx __user *buffer)
 {
-   return (__put_user(kts->tv_sec, >tv_sec) ||
-   __put_user(kts->tv_nsec,>tv_nsec   ) ||
-   __put_user(0,   >__reserved));
-}
-
-/*
- * Set the statx results.
- */
-static long statx_set_result(struct kstat *stat, struct statx __user *buffer)
-{
-   uid_t uid = from_kuid_munged(current_user_ns(), stat->uid);
-   gid_t gid = from_kgid_munged(current_user_ns(), stat->gid);
-
-   if (__put_user(stat->result_mask,   >stx_mask   ) ||
-   __put_user(stat->mode,  >stx_mode   ) ||
-   __clear_user(>__spare0, sizeof(buffer->__spare0)) ||
-   __put_user(stat->nlink, >stx_nlink  ) ||
-   __put_user(uid, >stx_uid) ||
-   __put_user(gid, >stx_gid) ||
-   __put_user(stat->attributes,>stx_attributes ) ||
-   __put_user(stat->blksize,   >stx_blksize) ||
-   __put_user(MAJOR(stat->rdev),   >stx_rdev_major ) ||
-   __put_user(MINOR(stat->rdev),   >stx_rdev_minor ) ||
-   __put_user(MAJOR(stat->dev),>stx_dev_major  ) ||
-   __put_user(MINOR(stat->dev),>stx_dev_minor  ) ||
-   __put_timestamp(>atime,   >stx_atime  ) ||
-   __put_timestamp(>btime,   >stx_btime  ) ||
-   __put_timestamp(>ctime,   >stx_ctime  ) ||
-   __put_timestamp(>mtime,   >stx_mtime  ) ||
-   __put_user(stat->ino,   >stx_ino) ||
-   __put_user(stat->size,  >stx_size   ) ||
-   __put_user(stat->blocks,>stx_blocks ) ||
-   __clear_user(>__spare1, sizeof(buffer->__spare1)) ||
-   __clear_user(>__spare2, sizeof(buffer->__spare2)))
-   return -EFAULT;
-
-   return 0;
+   struct statx tmp;
+
+   memset(, 0, sizeof(tmp));
+
+   tmp.stx_mask = stat->result_mask;
+   tmp.stx_blksize = stat->blksize;
+   tmp.stx_attributes = stat->attributes;
+   tmp.stx_nlink = stat->nlink;
+   tmp.stx_uid = from_kuid_munged(current_user_ns(), stat->uid);
+   tmp.stx_gid = from_kgid_munged(current_user_ns(), stat->gid);
+   tmp.stx_mode = stat->mode;
+   tmp.stx_ino = stat->ino;
+   tmp.stx_size = stat->size;
+   tmp.stx_blocks = stat->blocks;
+   tmp.stx_atime.tv_sec = stat->atime.tv_sec;
+   tmp.stx_atime.tv_nsec = stat->atime.tv_nsec;
+   tmp.stx_btime.tv_sec = stat->btime.tv_sec;
+   tmp.stx_btime.tv_nsec = stat->btime.tv_nsec;
+   tmp.stx_ctime.tv_sec = stat->ctime.tv_sec;

[PATCH v3] statx: optimize copy of struct statx to userspace

[Eric Biggers]

From: Eric Biggers 

I found that statx() was significantly slower than stat().  As a
microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
file to the same with statx() passed a NULL path:

$ time ./stat_benchmark

real0m1.464s
user0m0.275s
sys 0m1.187s

$ time ./statx_benchmark

real0m5.530s
user0m0.281s
sys 0m5.247s

statx is expected to be a little slower than stat because struct statx
is larger than struct stat, but not by *that* much.  It turns out that
most of the overhead was in copying struct statx to userspace, mostly in
all the stac/clac instructions that got generated for each __put_user()
call.  (This was on x86_64, but some other architectures, e.g. arm64,
have something similar now too.)

stat() instead initializes its struct on the stack and copies it to
userspace with a single call to copy_to_user().  This turns out to be
much faster, and changing statx to do this makes it almost as fast as
stat:

$ time ./statx_benchmark

real0m1.624s
user0m0.270s
sys 0m1.354s

Implementation-wise, for zeroing the reserved fields, I chose to simply
start by zeroing the full struct with memset.  This makes it clear that
every byte copied to userspace is initialized, even implicit padding
bytes (though there are none currently).  In the scenarios I tested, it
also performed the same as a designated initializer.  Manually
initializing each field was still slightly faster, but would have been
more error-prone and less verifiable.

Signed-off-by: Eric Biggers 
---
 fs/stat.c | 73 +++
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/fs/stat.c b/fs/stat.c
index fa0be59340cc..3fbecbfa6975 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -509,46 +509,36 @@ SYSCALL_DEFINE4(fstatat64, int, dfd, const char __user *, 
filename,
 }
 #endif /* __ARCH_WANT_STAT64 || __ARCH_WANT_COMPAT_STAT64 */
 
-static inline int __put_timestamp(struct timespec *kts,
- struct statx_timestamp __user *uts)
+static int cp_statx(const struct kstat *stat, struct statx __user *buffer)
 {
-   return (__put_user(kts->tv_sec, >tv_sec) ||
-   __put_user(kts->tv_nsec,>tv_nsec   ) ||
-   __put_user(0,   >__reserved));
-}
-
-/*
- * Set the statx results.
- */
-static long statx_set_result(struct kstat *stat, struct statx __user *buffer)
-{
-   uid_t uid = from_kuid_munged(current_user_ns(), stat->uid);
-   gid_t gid = from_kgid_munged(current_user_ns(), stat->gid);
-
-   if (__put_user(stat->result_mask,   >stx_mask   ) ||
-   __put_user(stat->mode,  >stx_mode   ) ||
-   __clear_user(>__spare0, sizeof(buffer->__spare0)) ||
-   __put_user(stat->nlink, >stx_nlink  ) ||
-   __put_user(uid, >stx_uid) ||
-   __put_user(gid, >stx_gid) ||
-   __put_user(stat->attributes,>stx_attributes ) ||
-   __put_user(stat->blksize,   >stx_blksize) ||
-   __put_user(MAJOR(stat->rdev),   >stx_rdev_major ) ||
-   __put_user(MINOR(stat->rdev),   >stx_rdev_minor ) ||
-   __put_user(MAJOR(stat->dev),>stx_dev_major  ) ||
-   __put_user(MINOR(stat->dev),>stx_dev_minor  ) ||
-   __put_timestamp(>atime,   >stx_atime  ) ||
-   __put_timestamp(>btime,   >stx_btime  ) ||
-   __put_timestamp(>ctime,   >stx_ctime  ) ||
-   __put_timestamp(>mtime,   >stx_mtime  ) ||
-   __put_user(stat->ino,   >stx_ino) ||
-   __put_user(stat->size,  >stx_size   ) ||
-   __put_user(stat->blocks,>stx_blocks ) ||
-   __clear_user(>__spare1, sizeof(buffer->__spare1)) ||
-   __clear_user(>__spare2, sizeof(buffer->__spare2)))
-   return -EFAULT;
-
-   return 0;
+   struct statx tmp;
+
+   memset(, 0, sizeof(tmp));
+
+   tmp.stx_mask = stat->result_mask;
+   tmp.stx_blksize = stat->blksize;
+   tmp.stx_attributes = stat->attributes;
+   tmp.stx_nlink = stat->nlink;
+   tmp.stx_uid = from_kuid_munged(current_user_ns(), stat->uid);
+   tmp.stx_gid = from_kgid_munged(current_user_ns(), stat->gid);
+   tmp.stx_mode = stat->mode;
+   tmp.stx_ino = stat->ino;
+   tmp.stx_size = stat->size;
+   tmp.stx_blocks = stat->blocks;
+   tmp.stx_atime.tv_sec = stat->atime.tv_sec;
+   tmp.stx_atime.tv_nsec = stat->atime.tv_nsec;
+   tmp.stx_btime.tv_sec = stat->btime.tv_sec;
+   tmp.stx_btime.tv_nsec = stat->btime.tv_nsec;
+   tmp.stx_ctime.tv_sec = stat->ctime.tv_sec;
+   tmp.stx_ctime.tv_nsec = 

Re: [RFC/PATCH] of: Mark property::value as const

[Frank Rowand]

Hi Stephen,

On 02/23/17 15:08, Frank Rowand wrote:
> On 02/13/17 18:50, Stephen Boyd wrote:
>> The 'blob' we pass into populate_properties() is marked as const,
>> but we cast that const away when we assign the result of
>> fdt_getprop_by_offset() to pp->value. Let's mark value as const
>> instead, so that code can't mistakenly write to the value of the
>> property that we've so far advertised as const.
> 
> Instead of struct property field value being a pointer into the
> FDT, I would rather copy the data to newly allocated memory and
> have value be a pointer to that memory.  This is required if we
> want to make /sys/firmware/fdt optional, which would allow us to
> free the memory containing the initial boot FDT.
> 
> I also do not want overlay live subtrees to have any pointers
> into the FDT that was used to populate the overlay, so copying
> the data solves that problem also.
> 
> 
>> Unfortunately, this exposes a problem with the fdt resolver code,
>> where we overwrite the value member of properties of phandles to
>> update them with their final value. Add a comment for now to
>> indicate where we're potentially writing over const data.
> 
> Yes, the resolver code needs to adjust phandle values.
> 
> I think I can get rid of the resolver modifying the various phandle
> values, and instead just modify the phandle value in struct
> device_node.  At the same time, I think I can also remove all
> instances of the phandle properties ('linux,phandle', 'ibm,phandle',
> 'phandle') in the live tree.  These properties should not be
> accessed directly by any code outside of the device tree framework
> since the phandle is located in the struct device_node.  A quick
> grep does not show any such accesses of the phandle properties,
> but I want to look more closely.

After reading through a bit of code, I am confident that I can
modify the resolver code to not modify the various phandle
property values.  There are a few tentacles reaching out to
other areas that I will have to fix also.  The biggest task
for me will be to create some good test code.

I'll be unavailable this week, so I'll start on it in about
a week.

-Frank

Re: [RFC/PATCH] of: Mark property::value as const

[Frank Rowand]

Hi Stephen,

On 02/23/17 15:08, Frank Rowand wrote:
> On 02/13/17 18:50, Stephen Boyd wrote:
>> The 'blob' we pass into populate_properties() is marked as const,
>> but we cast that const away when we assign the result of
>> fdt_getprop_by_offset() to pp->value. Let's mark value as const
>> instead, so that code can't mistakenly write to the value of the
>> property that we've so far advertised as const.
> 
> Instead of struct property field value being a pointer into the
> FDT, I would rather copy the data to newly allocated memory and
> have value be a pointer to that memory.  This is required if we
> want to make /sys/firmware/fdt optional, which would allow us to
> free the memory containing the initial boot FDT.
> 
> I also do not want overlay live subtrees to have any pointers
> into the FDT that was used to populate the overlay, so copying
> the data solves that problem also.
> 
> 
>> Unfortunately, this exposes a problem with the fdt resolver code,
>> where we overwrite the value member of properties of phandles to
>> update them with their final value. Add a comment for now to
>> indicate where we're potentially writing over const data.
> 
> Yes, the resolver code needs to adjust phandle values.
> 
> I think I can get rid of the resolver modifying the various phandle
> values, and instead just modify the phandle value in struct
> device_node.  At the same time, I think I can also remove all
> instances of the phandle properties ('linux,phandle', 'ibm,phandle',
> 'phandle') in the live tree.  These properties should not be
> accessed directly by any code outside of the device tree framework
> since the phandle is located in the struct device_node.  A quick
> grep does not show any such accesses of the phandle properties,
> but I want to look more closely.

After reading through a bit of code, I am confident that I can
modify the resolver code to not modify the various phandle
property values.  There are a few tentacles reaching out to
other areas that I will have to fix also.  The biggest task
for me will be to create some good test code.

I'll be unavailable this week, so I'll start on it in about
a week.

-Frank

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Eric Biggers]

On Sat, Mar 11, 2017 at 08:02:06PM -0800, Eric Biggers wrote:
> On Sun, Mar 12, 2017 at 02:29:27AM +, Al Viro wrote:
> > 
> > Oh, I agree that multiple __put_user() are wrong; I also agree that bulk 
> > copy is
> > the right approach (when we get the unsafe stuff right, we can revisit 
> > that, but
> > I suspect that on quite a few architectures a bulk copy will still give 
> > better
> > time, no matter what).
> > 
> > > If padding is a concern at all (AFAICS it's not actually an issue now with
> > > struct statx, but people tend to have different opinions on how careful 
> > > they
> > > want to be with padding), then I think we'll just have to start by 
> > > memsetting
> > > the whole struct to 0.
> > 
> > My point is simply that it's worth a comment in that code.
> 
> Okay, thanks.  I'll add a comment about the padding assumption, and I think 
> I'll
> take the suggestion to use a designated initializer.  Then at least all 
> *fields*
> get initialized by default.  And if in the future someone wants to 
> conditionally
> initialize fields, then they can use ?: or they can do it after the 
> initializer.
> Either way, at least they won't be able to forget to zero some field.
> 
> - Eric

Okay, well, I may have changed my mind again...  I tried the designated
initializer on x86_64 with gcc 4.8 and 6.3, and also on arm64 with gcc 4.8.  In
each case, it was compiled into first zeroing all 256 bytes of the struct, just
like memset(, 0, sizeof(tmp)).  Yes, this was with
CC_OPTIMIZE_FOR_PERFORMANCE=y.  So I think we might as well just write the full
memset(), making it completely clear that everything is initialized.  (This is
especially useful for people who are auditing code paths like this for
information leaks.)  Also, a smart compiler could potentially optimize away
parts of the memset() anyway...

Eric

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Eric Biggers]

On Sat, Mar 11, 2017 at 08:02:06PM -0800, Eric Biggers wrote:
> On Sun, Mar 12, 2017 at 02:29:27AM +, Al Viro wrote:
> > 
> > Oh, I agree that multiple __put_user() are wrong; I also agree that bulk 
> > copy is
> > the right approach (when we get the unsafe stuff right, we can revisit 
> > that, but
> > I suspect that on quite a few architectures a bulk copy will still give 
> > better
> > time, no matter what).
> > 
> > > If padding is a concern at all (AFAICS it's not actually an issue now with
> > > struct statx, but people tend to have different opinions on how careful 
> > > they
> > > want to be with padding), then I think we'll just have to start by 
> > > memsetting
> > > the whole struct to 0.
> > 
> > My point is simply that it's worth a comment in that code.
> 
> Okay, thanks.  I'll add a comment about the padding assumption, and I think 
> I'll
> take the suggestion to use a designated initializer.  Then at least all 
> *fields*
> get initialized by default.  And if in the future someone wants to 
> conditionally
> initialize fields, then they can use ?: or they can do it after the 
> initializer.
> Either way, at least they won't be able to forget to zero some field.
> 
> - Eric

Okay, well, I may have changed my mind again...  I tried the designated
initializer on x86_64 with gcc 4.8 and 6.3, and also on arm64 with gcc 4.8.  In
each case, it was compiled into first zeroing all 256 bytes of the struct, just
like memset(, 0, sizeof(tmp)).  Yes, this was with
CC_OPTIMIZE_FOR_PERFORMANCE=y.  So I think we might as well just write the full
memset(), making it completely clear that everything is initialized.  (This is
especially useful for people who are auditing code paths like this for
information leaks.)  Also, a smart compiler could potentially optimize away
parts of the memset() anyway...

Eric

[PATCH v3 3/3] iommu/ipmmu-vmsa: Hook up r8a7796 DT matching code

[Magnus Damm]

From: Magnus Damm 

Support the r8a7796 IPMMU by sharing feature flags between
r8a7795 and r8a7796. Also update IOMMU_OF_DECLARE to hook
up the updated compat string.

Signed-off-by: Magnus Damm 
---

 Changes since V2:
 - Updated to include white list suppport

 Changes since V1:
 - None

 drivers/iommu/ipmmu-vmsa.c |   14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

--- 0024/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-12 14:14:32.550607110 +0900
@@ -1058,8 +1058,9 @@ static bool ipmmu_slave_whitelist(struct
return false;
 }
 
-static const struct soc_device_attribute soc_r8a7795[] = {
+static const struct soc_device_attribute soc_rcar_gen3[] = {
{ .soc_id = "r8a7795", },
+   { .soc_id = "r8a7796", },
{ /* sentinel */ }
 };
 
@@ -1082,7 +1083,7 @@ static int ipmmu_of_xlate_dma(struct dev
}
 
/* For R-Car Gen3 use a white list to opt-in slave devices */
-   if (soc_device_match(soc_r8a7795) && !ipmmu_slave_whitelist(dev))
+   if (soc_device_match(soc_rcar_gen3) && !ipmmu_slave_whitelist(dev))
return -ENODEV;
 
return ipmmu_init_platform_device(dev);
@@ -1127,7 +1128,7 @@ static const struct ipmmu_features ipmmu
.twobit_imttbcr_sl0 = false,
 };
 
-static const struct ipmmu_features ipmmu_features_r8a7795 = {
+static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
.use_ns_alias_offset = false,
.has_cache_leaf_nodes = true,
.has_eight_ctx = true,
@@ -1141,7 +1142,10 @@ static const struct of_device_id ipmmu_o
.data = _features_default,
}, {
.compatible = "renesas,ipmmu-r8a7795",
-   .data = _features_r8a7795,
+   .data = _features_rcar_gen3,
+   }, {
+   .compatible = "renesas,ipmmu-r8a7796",
+   .data = _features_rcar_gen3,
}, {
/* Terminator */
},
@@ -1333,6 +1337,8 @@ IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "r
 ipmmu_vmsa_iommu_of_setup);
 IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795",
 ipmmu_vmsa_iommu_of_setup);
+IOMMU_OF_DECLARE(ipmmu_r8a7796_iommu_of, "renesas,ipmmu-r8a7796",
+ipmmu_vmsa_iommu_of_setup);
 #endif
 
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");

[PATCH v3 3/3] iommu/ipmmu-vmsa: Hook up r8a7796 DT matching code

[Magnus Damm]

From: Magnus Damm 

Support the r8a7796 IPMMU by sharing feature flags between
r8a7795 and r8a7796. Also update IOMMU_OF_DECLARE to hook
up the updated compat string.

Signed-off-by: Magnus Damm 
---

 Changes since V2:
 - Updated to include white list suppport

 Changes since V1:
 - None

 drivers/iommu/ipmmu-vmsa.c |   14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

--- 0024/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-12 14:14:32.550607110 +0900
@@ -1058,8 +1058,9 @@ static bool ipmmu_slave_whitelist(struct
return false;
 }
 
-static const struct soc_device_attribute soc_r8a7795[] = {
+static const struct soc_device_attribute soc_rcar_gen3[] = {
{ .soc_id = "r8a7795", },
+   { .soc_id = "r8a7796", },
{ /* sentinel */ }
 };
 
@@ -1082,7 +1083,7 @@ static int ipmmu_of_xlate_dma(struct dev
}
 
/* For R-Car Gen3 use a white list to opt-in slave devices */
-   if (soc_device_match(soc_r8a7795) && !ipmmu_slave_whitelist(dev))
+   if (soc_device_match(soc_rcar_gen3) && !ipmmu_slave_whitelist(dev))
return -ENODEV;
 
return ipmmu_init_platform_device(dev);
@@ -1127,7 +1128,7 @@ static const struct ipmmu_features ipmmu
.twobit_imttbcr_sl0 = false,
 };
 
-static const struct ipmmu_features ipmmu_features_r8a7795 = {
+static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
.use_ns_alias_offset = false,
.has_cache_leaf_nodes = true,
.has_eight_ctx = true,
@@ -1141,7 +1142,10 @@ static const struct of_device_id ipmmu_o
.data = _features_default,
}, {
.compatible = "renesas,ipmmu-r8a7795",
-   .data = _features_r8a7795,
+   .data = _features_rcar_gen3,
+   }, {
+   .compatible = "renesas,ipmmu-r8a7796",
+   .data = _features_rcar_gen3,
}, {
/* Terminator */
},
@@ -1333,6 +1337,8 @@ IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "r
 ipmmu_vmsa_iommu_of_setup);
 IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795",
 ipmmu_vmsa_iommu_of_setup);
+IOMMU_OF_DECLARE(ipmmu_r8a7796_iommu_of, "renesas,ipmmu-r8a7796",
+ipmmu_vmsa_iommu_of_setup);
 #endif
 
 MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");

[PATCH v3 2/3] iommu/ipmmu-vmsa: Increase maximum micro-TLBS to 48

[Magnus Damm]

From: Magnus Damm 

Bump up the maximum numbers of micro-TLBS to 48.

Each IPMMU device instance get micro-TLB assignment via
the "iommus" property in DT. Older SoCs tend to use a
maximum number of 32 micro-TLBs per IPMMU instance however
newer SoCs such as r8a7796 make use of up to 48 micro-TLBs.

At this point no SoC specific handling is done to validate
the maximum number of micro-TLBs, and because of that the
DT information is assumed to be within correct range for
each particular SoC.

If needed in the future SoC specific feature flags can be
added to handle the maximum number of micro-TLBs without
requiring DT changes, however at this point this does not
seem necessary.

Signed-off-by: Magnus Damm 
Reviewed-by: Geert Uytterhoeven 
---

 Changes since V2:
 - Added outer set of () to IMUASID() and IMUCTR() - thanks Ramesh!
 - Added Reviewed-by from Geert - thanks!

 Changes since V1:
 - Added support for the second I/O range at 0x600.

 drivers/iommu/ipmmu-vmsa.c |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--- 0020/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-12 14:11:23.020607110 +0900
@@ -213,7 +213,9 @@ static void set_archdata(struct device *
 #define IMPMBA(n)  (0x0280 + ((n) * 4))
 #define IMPMBD(n)  (0x02c0 + ((n) * 4))
 
-#define IMUCTR(n)  (0x0300 + ((n) * 16))
+#define IMUCTR(n)  ((n) < 32 ? IMUCTR0(n) : IMUCTR32(n))
+#define IMUCTR0(n) (0x0300 + ((n) * 16))
+#define IMUCTR32(n)(0x0600 + (((n) - 32) * 16))
 #define IMUCTR_FIXADDEN(1 << 31)
 #define IMUCTR_FIXADD_MASK (0xff << 16)
 #define IMUCTR_FIXADD_SHIFT16
@@ -223,7 +225,9 @@ static void set_archdata(struct device *
 #define IMUCTR_FLUSH   (1 << 1)
 #define IMUCTR_MMUEN   (1 << 0)
 
-#define IMUASID(n) (0x0308 + ((n) * 16))
+#define IMUASID(n) ((n) < 32 ? IMUASID0(n) : IMUASID32(n))
+#define IMUASID0(n)(0x0308 + ((n) * 16))
+#define IMUASID32(n)   (0x0608 + (((n) - 32) * 16))
 #define IMUASID_ASID8_MASK (0xff << 8)
 #define IMUASID_ASID8_SHIFT8
 #define IMUASID_ASID0_MASK (0xff << 0)
@@ -1164,7 +1168,7 @@ static int ipmmu_probe(struct platform_d
}
 
mmu->dev = >dev;
-   mmu->num_utlbs = 32;
+   mmu->num_utlbs = 48;
spin_lock_init(>lock);
bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
mmu->features = match->data;

[PATCH v3 2/3] iommu/ipmmu-vmsa: Increase maximum micro-TLBS to 48

[Magnus Damm]

From: Magnus Damm 

Bump up the maximum numbers of micro-TLBS to 48.

Each IPMMU device instance get micro-TLB assignment via
the "iommus" property in DT. Older SoCs tend to use a
maximum number of 32 micro-TLBs per IPMMU instance however
newer SoCs such as r8a7796 make use of up to 48 micro-TLBs.

At this point no SoC specific handling is done to validate
the maximum number of micro-TLBs, and because of that the
DT information is assumed to be within correct range for
each particular SoC.

If needed in the future SoC specific feature flags can be
added to handle the maximum number of micro-TLBs without
requiring DT changes, however at this point this does not
seem necessary.

Signed-off-by: Magnus Damm 
Reviewed-by: Geert Uytterhoeven 
---

 Changes since V2:
 - Added outer set of () to IMUASID() and IMUCTR() - thanks Ramesh!
 - Added Reviewed-by from Geert - thanks!

 Changes since V1:
 - Added support for the second I/O range at 0x600.

 drivers/iommu/ipmmu-vmsa.c |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--- 0020/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2017-03-12 14:11:23.020607110 +0900
@@ -213,7 +213,9 @@ static void set_archdata(struct device *
 #define IMPMBA(n)  (0x0280 + ((n) * 4))
 #define IMPMBD(n)  (0x02c0 + ((n) * 4))
 
-#define IMUCTR(n)  (0x0300 + ((n) * 16))
+#define IMUCTR(n)  ((n) < 32 ? IMUCTR0(n) : IMUCTR32(n))
+#define IMUCTR0(n) (0x0300 + ((n) * 16))
+#define IMUCTR32(n)(0x0600 + (((n) - 32) * 16))
 #define IMUCTR_FIXADDEN(1 << 31)
 #define IMUCTR_FIXADD_MASK (0xff << 16)
 #define IMUCTR_FIXADD_SHIFT16
@@ -223,7 +225,9 @@ static void set_archdata(struct device *
 #define IMUCTR_FLUSH   (1 << 1)
 #define IMUCTR_MMUEN   (1 << 0)
 
-#define IMUASID(n) (0x0308 + ((n) * 16))
+#define IMUASID(n) ((n) < 32 ? IMUASID0(n) : IMUASID32(n))
+#define IMUASID0(n)(0x0308 + ((n) * 16))
+#define IMUASID32(n)   (0x0608 + (((n) - 32) * 16))
 #define IMUASID_ASID8_MASK (0xff << 8)
 #define IMUASID_ASID8_SHIFT8
 #define IMUASID_ASID0_MASK (0xff << 0)
@@ -1164,7 +1168,7 @@ static int ipmmu_probe(struct platform_d
}
 
mmu->dev = >dev;
-   mmu->num_utlbs = 32;
+   mmu->num_utlbs = 48;
spin_lock_init(>lock);
bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
mmu->features = match->data;

[PATCH v3 0/3] iommu/ipmmu-vmsa: r8a7796 support V3

[Magnus Damm]

iommu/ipmmu-vmsa: r8a7796 support V3

[PATCH v3 1/3] iommu/ipmmu-vmsa: Add r8a7796 DT binding
[PATCH v3 2/3] iommu/ipmmu-vmsa: Increase maximum micro-TLBS to 48
[PATCH v3 3/3] iommu/ipmmu-vmsa: Hook up r8a7796 DT matching code

This series adds r8a7796 support to the IPMMU driver. The DT binding
gets updated, maximum number of micro-TLBs are increased and the
driver is adjusted to match on the new DT binding.

Changes since V2:
 - Patch 2/3 updated with an outer set of () - thanks Ramesh!
 - Patch 2/3 updated with Reviewed-by - thanks Geert!
 - Patch 3/3 updated to include white list support

Changes since V1:
 - Patch 1/3 updated with more Acked-by tags
 - Patch 2/3 updated with high I/O register range support

Patch 1/3 is ready for upstream merge and includes the following tags:
Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
Acked-by: Rob Herring 
Acked-by: Simon Horman 
Acked-by: Geert Uytterhoeven 

Patch 2/3 and 3/3 are quite trivial but have now acked-by so far.

Signed-off-by: Magnus Damm 
---

 Developed on top of v4.11-rc1 with the following series applied:
 [PATCH v7 00/07] iommu/ipmmu-vmsa: IPMMU multi-arch update V7
 [PATCH v3 00/09] iommu/ipmmu-vmsa: r8a7795 support V3

 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |1 
 drivers/iommu/ipmmu-vmsa.c |   24 
+++---
 2 files changed, 18 insertions(+), 7 deletions(-)

[PATCH v3 1/3] iommu/ipmmu-vmsa: Add r8a7796 DT binding

[Magnus Damm]

From: Magnus Damm 

Update the IPMMU DT binding documentation to include the r8a7796 compat
string for R-Car M3-W.

Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
Acked-by: Rob Herring 
Acked-by: Simon Horman 
Acked-by: Geert Uytterhoeven 
---

 Changes since V2:
 - None

 Changes since V1:
 - Included in series, added Acked-by from Geert - thanks!

 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |1 +
 1 file changed, 1 insertion(+)

--- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
+++ work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 
2017-03-12 14:09:01.920607110 +0900
@@ -16,6 +16,7 @@ Required Properties:
 - "renesas,ipmmu-r8a7793" for the R8A7793 (R-Car M2-N) IPMMU.
 - "renesas,ipmmu-r8a7794" for the R8A7794 (R-Car E2) IPMMU.
 - "renesas,ipmmu-r8a7795" for the R8A7795 (R-Car H3) IPMMU.
+- "renesas,ipmmu-r8a7796" for the R8A7796 (R-Car M3-W) IPMMU.
 - "renesas,ipmmu-vmsa" for generic R-Car Gen2 VMSA-compatible IPMMU.
 
   - reg: Base address and size of the IPMMU registers.

[PATCH v3 0/3] iommu/ipmmu-vmsa: r8a7796 support V3

[Magnus Damm]

iommu/ipmmu-vmsa: r8a7796 support V3

[PATCH v3 1/3] iommu/ipmmu-vmsa: Add r8a7796 DT binding
[PATCH v3 2/3] iommu/ipmmu-vmsa: Increase maximum micro-TLBS to 48
[PATCH v3 3/3] iommu/ipmmu-vmsa: Hook up r8a7796 DT matching code

This series adds r8a7796 support to the IPMMU driver. The DT binding
gets updated, maximum number of micro-TLBs are increased and the
driver is adjusted to match on the new DT binding.

Changes since V2:
 - Patch 2/3 updated with an outer set of () - thanks Ramesh!
 - Patch 2/3 updated with Reviewed-by - thanks Geert!
 - Patch 3/3 updated to include white list support

Changes since V1:
 - Patch 1/3 updated with more Acked-by tags
 - Patch 2/3 updated with high I/O register range support

Patch 1/3 is ready for upstream merge and includes the following tags:
Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
Acked-by: Rob Herring 
Acked-by: Simon Horman 
Acked-by: Geert Uytterhoeven 

Patch 2/3 and 3/3 are quite trivial but have now acked-by so far.

Signed-off-by: Magnus Damm 
---

 Developed on top of v4.11-rc1 with the following series applied:
 [PATCH v7 00/07] iommu/ipmmu-vmsa: IPMMU multi-arch update V7
 [PATCH v3 00/09] iommu/ipmmu-vmsa: r8a7795 support V3

 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |1 
 drivers/iommu/ipmmu-vmsa.c |   24 
+++---
 2 files changed, 18 insertions(+), 7 deletions(-)

[PATCH v3 1/3] iommu/ipmmu-vmsa: Add r8a7796 DT binding

[Magnus Damm]

From: Magnus Damm 

Update the IPMMU DT binding documentation to include the r8a7796 compat
string for R-Car M3-W.

Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
Acked-by: Rob Herring 
Acked-by: Simon Horman 
Acked-by: Geert Uytterhoeven 
---

 Changes since V2:
 - None

 Changes since V1:
 - Included in series, added Acked-by from Geert - thanks!

 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |1 +
 1 file changed, 1 insertion(+)

--- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
+++ work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 
2017-03-12 14:09:01.920607110 +0900
@@ -16,6 +16,7 @@ Required Properties:
 - "renesas,ipmmu-r8a7793" for the R8A7793 (R-Car M2-N) IPMMU.
 - "renesas,ipmmu-r8a7794" for the R8A7794 (R-Car E2) IPMMU.
 - "renesas,ipmmu-r8a7795" for the R8A7795 (R-Car H3) IPMMU.
+- "renesas,ipmmu-r8a7796" for the R8A7796 (R-Car M3-W) IPMMU.
 - "renesas,ipmmu-vmsa" for generic R-Car Gen2 VMSA-compatible IPMMU.
 
   - reg: Base address and size of the IPMMU registers.

[BUG] 4.11.0-rc1 panic on shutdown X61s

[lkml]

Hello list,

Here's a photo of the panic, on imgur to be kind to vger:
http://imgur.com/a/wZI32

I'm out on a sailboat so can't really do much, but had a chance with internet
to send this FYI.  I don't even know if this happens always or not yet.

Never seen this before, up to and including 4.10.0.

Regards,
Vito Caputo

[BUG] 4.11.0-rc1 panic on shutdown X61s

[lkml]

Hello list,

Here's a photo of the panic, on imgur to be kind to vger:
http://imgur.com/a/wZI32

I'm out on a sailboat so can't really do much, but had a chance with internet
to send this FYI.  I don't even know if this happens always or not yet.

Never seen this before, up to and including 4.10.0.

Regards,
Vito Caputo

[PATCH] locking/hung_task: Defer showing held locks

[Tetsuo Handa]

When I was running my testcase which may block hundreds of threads
on fs locks, I got lockup due to output from debug_show_all_locks()
added by commit b2d4c2edb2e4f89a ("locking/hung_task: Show all locks").

For example, if 1000 threads were blocked in TASK_UNINTERRUPTIBLE state
and 500 out of 1000 threads hold some lock, debug_show_all_locks() from
for_each_process_thread() loop will report locks held by 500 threads for
1000 times. This is a too much noise.

In order to make sure rcu_lock_break() is called frequently, we should
avoid calling debug_show_all_locks() from for_each_process_thread() loop
because debug_show_all_locks() effectively calls for_each_process_thread()
loop. Let's defer calling debug_show_all_locks() till before panic() or
leaving for_each_process_thread() loop.

Signed-off-by: Tetsuo Handa 
Cc: Vegard Nossum 
---
 kernel/hung_task.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index f0f8e2a..751593e 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -43,6 +43,7 @@
 int __read_mostly sysctl_hung_task_warnings = 10;
 
 static int __read_mostly did_panic;
+static bool hung_task_show_lock;
 
 static struct task_struct *watchdog_task;
 
@@ -120,12 +121,14 @@ static void check_hung_task(struct task_struct *t, 
unsigned long timeout)
pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
" disables this message.\n");
sched_show_task(t);
-   debug_show_all_locks();
+   hung_task_show_lock = true;
}
 
touch_nmi_watchdog();
 
if (sysctl_hung_task_panic) {
+   if (hung_task_show_lock)
+   debug_show_all_locks();
trigger_all_cpu_backtrace();
panic("hung_task: blocked tasks");
}
@@ -172,6 +175,7 @@ static void check_hung_uninterruptible_tasks(unsigned long 
timeout)
if (test_taint(TAINT_DIE) || did_panic)
return;
 
+   hung_task_show_lock = false;
rcu_read_lock();
for_each_process_thread(g, t) {
if (!max_count--)
@@ -187,6 +191,8 @@ static void check_hung_uninterruptible_tasks(unsigned long 
timeout)
}
  unlock:
rcu_read_unlock();
+   if (hung_task_show_lock)
+   debug_show_all_locks();
 }
 
 static long hung_timeout_jiffies(unsigned long last_checked,
-- 
1.8.3.1

[PATCH] locking/hung_task: Defer showing held locks

[Tetsuo Handa]

When I was running my testcase which may block hundreds of threads
on fs locks, I got lockup due to output from debug_show_all_locks()
added by commit b2d4c2edb2e4f89a ("locking/hung_task: Show all locks").

For example, if 1000 threads were blocked in TASK_UNINTERRUPTIBLE state
and 500 out of 1000 threads hold some lock, debug_show_all_locks() from
for_each_process_thread() loop will report locks held by 500 threads for
1000 times. This is a too much noise.

In order to make sure rcu_lock_break() is called frequently, we should
avoid calling debug_show_all_locks() from for_each_process_thread() loop
because debug_show_all_locks() effectively calls for_each_process_thread()
loop. Let's defer calling debug_show_all_locks() till before panic() or
leaving for_each_process_thread() loop.

Signed-off-by: Tetsuo Handa 
Cc: Vegard Nossum 
---
 kernel/hung_task.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index f0f8e2a..751593e 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -43,6 +43,7 @@
 int __read_mostly sysctl_hung_task_warnings = 10;
 
 static int __read_mostly did_panic;
+static bool hung_task_show_lock;
 
 static struct task_struct *watchdog_task;
 
@@ -120,12 +121,14 @@ static void check_hung_task(struct task_struct *t, 
unsigned long timeout)
pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
" disables this message.\n");
sched_show_task(t);
-   debug_show_all_locks();
+   hung_task_show_lock = true;
}
 
touch_nmi_watchdog();
 
if (sysctl_hung_task_panic) {
+   if (hung_task_show_lock)
+   debug_show_all_locks();
trigger_all_cpu_backtrace();
panic("hung_task: blocked tasks");
}
@@ -172,6 +175,7 @@ static void check_hung_uninterruptible_tasks(unsigned long 
timeout)
if (test_taint(TAINT_DIE) || did_panic)
return;
 
+   hung_task_show_lock = false;
rcu_read_lock();
for_each_process_thread(g, t) {
if (!max_count--)
@@ -187,6 +191,8 @@ static void check_hung_uninterruptible_tasks(unsigned long 
timeout)
}
  unlock:
rcu_read_unlock();
+   if (hung_task_show_lock)
+   debug_show_all_locks();
 }
 
 static long hung_timeout_jiffies(unsigned long last_checked,
-- 
1.8.3.1

Re: [PATCH 4.10 000/167] 4.10.2-stable review

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 03:24:52PM -0800, Kevin Hilman wrote:
> kernelci.org bot  writes:
> 
> > stable-rc boot: 541 boots: 6 failed, 500 passed with 34 offline, 1 conflict 
> > (v4.10.1-168-gcdc1f9d24aac)
> >
> > Full Boot Summary: 
> > https://kernelci.org/boot/all/job/stable-rc/kernel/v4.10.1-168-gcdc1f9d24aac/
> > Full Build Summary: 
> > https://kernelci.org/build/stable-rc/kernel/v4.10.1-168-gcdc1f9d24aac/
> >
> > Tree: stable-rc
> > Branch: local/linux-4.10.y
> > Git Describe: v4.10.1-168-gcdc1f9d24aac
> > Git Commit: cdc1f9d24aac385a7fe4611d7b42f51e20f49cdb
> > Git URL: 
> > http://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > Tested: 101 unique boards, 25 SoC families, 30 builds out of 204
> >
> > Boot Regressions Detected:
> >
> > arm:
> >
> > multi_v7_defconfig+CONFIG_PROVE_LOCKING=y:
> > am335x-pepper:
> > lab-baylibre-seattle: new failure (last pass: 
> > v4.10-21-gd23a9821d397)
> 
> This one is a new regression, and a first attempt at bisect was
> inconclusive. 
> 
> > Boot Failures Detected:
> >
> > arm64:
> >
> > defconfig+CONFIG_RANDOMIZE_BASE=y
> > alpine-v2-evp: 1 failed lab
> 
> This one appears to be a new board in the Free Electrons lab, which
> doesn't have a history of passing.
> 
> Quentin, Antoine: blacklist or fix?
> 
> > apm-mustang: 1 failed lab
> > juno: 1 failed lab
> 
> These aren't new and have to do with broken boot firmware/UEFI that
> cannot cope with bigger kernels.  The folks in the Linaro Cambridge lab
> are looking into upgrading the firmware.
> 
> > arm:
> > qcom_defconfig
> > qcom-apq8064-cm-qs600: 1 failed lab
> > qcom-apq8064-ifc6410: 1 failed lab
> 
> These also exist in v4.10 mainline and have been reported to qcom
> maintainer Andy Gross (cc'd).

Thanks for the analysis of all of these, much appreciated.

greg k-h

Re: [PATCH 4.10 000/167] 4.10.2-stable review

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 03:24:52PM -0800, Kevin Hilman wrote:
> kernelci.org bot  writes:
> 
> > stable-rc boot: 541 boots: 6 failed, 500 passed with 34 offline, 1 conflict 
> > (v4.10.1-168-gcdc1f9d24aac)
> >
> > Full Boot Summary: 
> > https://kernelci.org/boot/all/job/stable-rc/kernel/v4.10.1-168-gcdc1f9d24aac/
> > Full Build Summary: 
> > https://kernelci.org/build/stable-rc/kernel/v4.10.1-168-gcdc1f9d24aac/
> >
> > Tree: stable-rc
> > Branch: local/linux-4.10.y
> > Git Describe: v4.10.1-168-gcdc1f9d24aac
> > Git Commit: cdc1f9d24aac385a7fe4611d7b42f51e20f49cdb
> > Git URL: 
> > http://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > Tested: 101 unique boards, 25 SoC families, 30 builds out of 204
> >
> > Boot Regressions Detected:
> >
> > arm:
> >
> > multi_v7_defconfig+CONFIG_PROVE_LOCKING=y:
> > am335x-pepper:
> > lab-baylibre-seattle: new failure (last pass: 
> > v4.10-21-gd23a9821d397)
> 
> This one is a new regression, and a first attempt at bisect was
> inconclusive. 
> 
> > Boot Failures Detected:
> >
> > arm64:
> >
> > defconfig+CONFIG_RANDOMIZE_BASE=y
> > alpine-v2-evp: 1 failed lab
> 
> This one appears to be a new board in the Free Electrons lab, which
> doesn't have a history of passing.
> 
> Quentin, Antoine: blacklist or fix?
> 
> > apm-mustang: 1 failed lab
> > juno: 1 failed lab
> 
> These aren't new and have to do with broken boot firmware/UEFI that
> cannot cope with bigger kernels.  The folks in the Linaro Cambridge lab
> are looking into upgrading the firmware.
> 
> > arm:
> > qcom_defconfig
> > qcom-apq8064-cm-qs600: 1 failed lab
> > qcom-apq8064-ifc6410: 1 failed lab
> 
> These also exist in v4.10 mainline and have been reported to qcom
> maintainer Andy Gross (cc'd).

Thanks for the analysis of all of these, much appreciated.

greg k-h

Re: [PATCH 4.10 000/167] 4.10.2-stable review

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 10:36:55AM -0800, Guenter Roeck wrote:
> On Fri, Mar 10, 2017 at 10:07:23AM +0100, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.10.2 release.
> > There are 167 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sun Mar 12 08:39:21 UTC 2017.
> > Anything received after that time might be too late.
> > 
> Build results:
>   total: 149 pass: 149 fail: 0
> Qemu test results:
>   total: 122 pass: 122 fail: 0
> 
> Details are available at http://kerneltests.org/builders.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 4.10 000/167] 4.10.2-stable review

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 10:36:55AM -0800, Guenter Roeck wrote:
> On Fri, Mar 10, 2017 at 10:07:23AM +0100, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.10.2 release.
> > There are 167 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sun Mar 12 08:39:21 UTC 2017.
> > Anything received after that time might be too late.
> > 
> Build results:
>   total: 149 pass: 149 fail: 0
> Qemu test results:
>   total: 122 pass: 122 fail: 0
> 
> Details are available at http://kerneltests.org/builders.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 4.4 48/91] ext4: fix inline data error paths

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 04:48:52PM +, Ben Hutchings wrote:
> On Fri, 2017-03-10 at 10:08 +0100, Greg Kroah-Hartman wrote:
> > 4.4-stable review patch.  If anyone has any objections, please let me
> > know.
> > 
> > --
> > 
> > From: Theodore Ts'o 
> > 
> > commit eb5efbcb762aee4b454b04f7115f73ccbcf8f0ef upstream.
> > 
> > The write_end() function must always unlock the page and drop its ref
> > count, even on an error.
> 
> This looks like a theoretical rather than a real issue, because I can't
> see how ext4_write_inline_data_end() ever returns an error code.

I'll leave that up to Ted to justify :)

> > Signed-off-by: Theodore Ts'o 
> > Signed-off-by: Greg Kroah-Hartman 
> > 
> > ---
> >  fs/ext4/inline.c |9 -
> >  fs/ext4/inode.c  |   20 +++-
> >  2 files changed, 23 insertions(+), 6 deletions(-)
> > 
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -933,8 +933,15 @@ int ext4_da_write_inline_data_end(struct
> > >     struct page *page)
> >  {
> >     int i_size_changed = 0;
> > +   int ret;
> >  
> > -   copied = ext4_write_inline_data_end(inode, pos, len, copied, page);
> > +   ret = ext4_write_inline_data_end(inode, pos, len, copied, page);
> > +   if (ret < 0) {
> > +   unlock_page(page);
> > +   put_page(page);
> [...]
> 
> For 4.4 each put_page() should ideally be changed to
> page_cache_release().  It makes no practical difference but would be
> consistent with other paths.

As it's still the same logic, I'd prefer to stick to what newer kernels
do if at all possible.

thanks,

greg k-h

Re: [PATCH 4.10 000/167] 4.10.2-stable review

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 12:14:35PM -0700, Shuah Khan wrote:
> On 03/10/2017 02:07 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.10.2 release.
> > There are 167 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sun Mar 12 08:39:21 UTC 2017.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.10.2-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.10.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 4.4 48/91] ext4: fix inline data error paths

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 04:48:52PM +, Ben Hutchings wrote:
> On Fri, 2017-03-10 at 10:08 +0100, Greg Kroah-Hartman wrote:
> > 4.4-stable review patch.  If anyone has any objections, please let me
> > know.
> > 
> > --
> > 
> > From: Theodore Ts'o 
> > 
> > commit eb5efbcb762aee4b454b04f7115f73ccbcf8f0ef upstream.
> > 
> > The write_end() function must always unlock the page and drop its ref
> > count, even on an error.
> 
> This looks like a theoretical rather than a real issue, because I can't
> see how ext4_write_inline_data_end() ever returns an error code.

I'll leave that up to Ted to justify :)

> > Signed-off-by: Theodore Ts'o 
> > Signed-off-by: Greg Kroah-Hartman 
> > 
> > ---
> >  fs/ext4/inline.c |9 -
> >  fs/ext4/inode.c  |   20 +++-
> >  2 files changed, 23 insertions(+), 6 deletions(-)
> > 
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -933,8 +933,15 @@ int ext4_da_write_inline_data_end(struct
> > >     struct page *page)
> >  {
> >     int i_size_changed = 0;
> > +   int ret;
> >  
> > -   copied = ext4_write_inline_data_end(inode, pos, len, copied, page);
> > +   ret = ext4_write_inline_data_end(inode, pos, len, copied, page);
> > +   if (ret < 0) {
> > +   unlock_page(page);
> > +   put_page(page);
> [...]
> 
> For 4.4 each put_page() should ideally be changed to
> page_cache_release().  It makes no practical difference but would be
> consistent with other paths.

As it's still the same logic, I'd prefer to stick to what newer kernels
do if at all possible.

thanks,

greg k-h

Re: [PATCH 4.10 000/167] 4.10.2-stable review

[Greg Kroah-Hartman]

On Fri, Mar 10, 2017 at 12:14:35PM -0700, Shuah Khan wrote:
> On 03/10/2017 02:07 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.10.2 release.
> > There are 167 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sun Mar 12 08:39:21 UTC 2017.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.10.2-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.10.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH] staging: sm750fb: Improved coding style

[Greg Kroah-Hartman]

On Sun, Mar 12, 2017 at 08:04:02AM +0530, Arushi Singhal wrote:
> New variables are added to make the code more readable.
> Also fixed the checkpatch issue: "Alignment should match open
> parenthesis".

When you have to say "also" in a commit message, that is a huge red-flag
that you need to break a patch up into different parts, as you are doing
multiple things.  Please do so here.

thanks,

greg k-h

Re: [PATCH] staging: sm750fb: Improved coding style

[Greg Kroah-Hartman]

On Sun, Mar 12, 2017 at 08:04:02AM +0530, Arushi Singhal wrote:
> New variables are added to make the code more readable.
> Also fixed the checkpatch issue: "Alignment should match open
> parenthesis".

When you have to say "also" in a commit message, that is a huge red-flag
that you need to break a patch up into different parts, as you are doing
multiple things.  Please do so here.

thanks,

greg k-h

Re: [PATCH v5] blkcg: allocate struct blkcg_gq outside request queue spinlock

[Tahsin Erdogan]

On Sat, Mar 11, 2017 at 2:52 PM, Jens Axboe  wrote:

>
> Talked to Tejun about this as well, and we both agree that the splitting
> this into separate init/alloc paths would be much cleaner. I can't
> apply the current patch, sorry, it's just too ugly to live.

Do you mean, you prefer the approach that was taken in v1 patch or
something else?

Re: [PATCH v5] blkcg: allocate struct blkcg_gq outside request queue spinlock

[Tahsin Erdogan]

On Sat, Mar 11, 2017 at 2:52 PM, Jens Axboe  wrote:

>
> Talked to Tejun about this as well, and we both agree that the splitting
> this into separate init/alloc paths would be much cleaner. I can't
> apply the current patch, sorry, it's just too ugly to live.

Do you mean, you prefer the approach that was taken in v1 patch or
something else?

Re: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Michael S. Tsirkin]

On Sun, Mar 12, 2017 at 01:59:54AM +, Wang, Wei W wrote:
> On 03/11/2017 10:10 PM, Matthew Wilcox wrote:
> > On Sat, Mar 11, 2017 at 07:59:31PM +0800, Wei Wang wrote:
> > > I'm thinking what if the guest needs to transfer these much physically
> > > continuous memory to host: 1GB+2MB+64KB+32KB+16KB+4KB.
> > > Is it going to use Six 64-bit chunks? Would it be simpler if we just
> > > use the 128-bit chunk format (we can drop the previous normal 64-bit
> > > format)?
> > 
> > Is that a likely thing for the guest to need to do though?  Freeing a 1GB 
> > page is
> > much more liikely, IMO.
> 
> Yes, I think it's very possible. The host can ask for any number of pages 
> (e.g. 1.5GB) that the guest can afford.  Also, the ballooned 1.5G memory is 
> not guaranteed to be continuous in any pattern like 1GB+512MB. That's why we 
> need to use a bitmap to draw the whole picture first, and then seek for 
> continuous bits to chunk.
> 
> Best,
> Wei

While I like the clever format that Matthew came up with, I'm also
inclined to say let's keep things simple.
the simplest thing seems to be to use the ext format all the time.
Except let's reserve the low 12 bits in both address and size,
since they are already 0, we might be able to use them for flags down the road.

-- 
MST

Re: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Michael S. Tsirkin]

On Sun, Mar 12, 2017 at 01:59:54AM +, Wang, Wei W wrote:
> On 03/11/2017 10:10 PM, Matthew Wilcox wrote:
> > On Sat, Mar 11, 2017 at 07:59:31PM +0800, Wei Wang wrote:
> > > I'm thinking what if the guest needs to transfer these much physically
> > > continuous memory to host: 1GB+2MB+64KB+32KB+16KB+4KB.
> > > Is it going to use Six 64-bit chunks? Would it be simpler if we just
> > > use the 128-bit chunk format (we can drop the previous normal 64-bit
> > > format)?
> > 
> > Is that a likely thing for the guest to need to do though?  Freeing a 1GB 
> > page is
> > much more liikely, IMO.
> 
> Yes, I think it's very possible. The host can ask for any number of pages 
> (e.g. 1.5GB) that the guest can afford.  Also, the ballooned 1.5G memory is 
> not guaranteed to be continuous in any pattern like 1GB+512MB. That's why we 
> need to use a bitmap to draw the whole picture first, and then seek for 
> continuous bits to chunk.
> 
> Best,
> Wei

While I like the clever format that Matthew came up with, I'm also
inclined to say let's keep things simple.
the simplest thing seems to be to use the ext format all the time.
Except let's reserve the low 12 bits in both address and size,
since they are already 0, we might be able to use them for flags down the road.

-- 
MST

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Eric Biggers]

On Sun, Mar 12, 2017 at 02:29:27AM +, Al Viro wrote:
> 
> Oh, I agree that multiple __put_user() are wrong; I also agree that bulk copy 
> is
> the right approach (when we get the unsafe stuff right, we can revisit that, 
> but
> I suspect that on quite a few architectures a bulk copy will still give better
> time, no matter what).
> 
> > If padding is a concern at all (AFAICS it's not actually an issue now with
> > struct statx, but people tend to have different opinions on how careful they
> > want to be with padding), then I think we'll just have to start by 
> > memsetting
> > the whole struct to 0.
> 
> My point is simply that it's worth a comment in that code.

Okay, thanks.  I'll add a comment about the padding assumption, and I think I'll
take the suggestion to use a designated initializer.  Then at least all *fields*
get initialized by default.  And if in the future someone wants to conditionally
initialize fields, then they can use ?: or they can do it after the initializer.
Either way, at least they won't be able to forget to zero some field.

- Eric

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Eric Biggers]

On Sun, Mar 12, 2017 at 02:29:27AM +, Al Viro wrote:
> 
> Oh, I agree that multiple __put_user() are wrong; I also agree that bulk copy 
> is
> the right approach (when we get the unsafe stuff right, we can revisit that, 
> but
> I suspect that on quite a few architectures a bulk copy will still give better
> time, no matter what).
> 
> > If padding is a concern at all (AFAICS it's not actually an issue now with
> > struct statx, but people tend to have different opinions on how careful they
> > want to be with padding), then I think we'll just have to start by 
> > memsetting
> > the whole struct to 0.
> 
> My point is simply that it's worth a comment in that code.

Okay, thanks.  I'll add a comment about the padding assumption, and I think I'll
take the suggestion to use a designated initializer.  Then at least all *fields*
get initialized by default.  And if in the future someone wants to conditionally
initialize fields, then they can use ?: or they can do it after the initializer.
Either way, at least they won't be able to forget to zero some field.

- Eric

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Steve Longerbeam]

On 03/11/2017 10:59 AM, Russell King - ARM Linux wrote:

On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:



On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:

I really don't think expecting the user to understand and configure
the pipeline is a sane way forward.  Think about it - should the
user need to know that, because they have a bayer-only CSI data
source, that there is only one path possible, and if they try to
configure a different path, then things will just error out?

For the case of imx219 connected to iMX6, it really is as simple as
"there is only one possible path" and all the complexity of the media
interfaces/subdevs is completely unnecessary.  Every other block in
the graph is just noise.

The fact is that these dot graphs show a complex picture, but reality
is somewhat different - there's only relatively few paths available
depending on the connected source and the rest of the paths are
completely useless.



I totally disagree there. Raw bayer requires passthrough yes, but for
all other media bus formats on a mipi csi-2 bus, and all other media
bus formats on 8-bit parallel buses, the conersion pipelines can be
used for scaling, CSC, rotation, and motion-compensated de-interlacing.


... which only makes sense _if_ your source can produce those formats.
We don't actually disagree on that.


...and there are lots of those sources! You should try getting out of
your imx219 shell some time, and have a look around! :)



Let me re-state.  If the source can _only_ produce bayer, then there is
_only_ _one_ possible path, and all the overhead of the media controller
stuff is totally unnecessary.

Or, are you going to tell me that the user should have the right to
configure paths through the iMX6 hardware that are not permitted by the
iMX6 manuals for the data format being produced by the sensor?


Anyway, no the user is not allowed to configure a path that is not
allowed by the hardware, such as attempting to pass raw bayer through
an Image Converter path.

I guess you are simply commenting that for users of bayer sensors, the
other pipelines can be "confusing". But I trust you're not saying those
other pipelines should therefore not be present, which would be a
completely nutty argument.

Steve

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Steve Longerbeam]

On 03/11/2017 10:59 AM, Russell King - ARM Linux wrote:

On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:



On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:

I really don't think expecting the user to understand and configure
the pipeline is a sane way forward.  Think about it - should the
user need to know that, because they have a bayer-only CSI data
source, that there is only one path possible, and if they try to
configure a different path, then things will just error out?

For the case of imx219 connected to iMX6, it really is as simple as
"there is only one possible path" and all the complexity of the media
interfaces/subdevs is completely unnecessary.  Every other block in
the graph is just noise.

The fact is that these dot graphs show a complex picture, but reality
is somewhat different - there's only relatively few paths available
depending on the connected source and the rest of the paths are
completely useless.



I totally disagree there. Raw bayer requires passthrough yes, but for
all other media bus formats on a mipi csi-2 bus, and all other media
bus formats on 8-bit parallel buses, the conersion pipelines can be
used for scaling, CSC, rotation, and motion-compensated de-interlacing.


... which only makes sense _if_ your source can produce those formats.
We don't actually disagree on that.


...and there are lots of those sources! You should try getting out of
your imx219 shell some time, and have a look around! :)



Let me re-state.  If the source can _only_ produce bayer, then there is
_only_ _one_ possible path, and all the overhead of the media controller
stuff is totally unnecessary.

Or, are you going to tell me that the user should have the right to
configure paths through the iMX6 hardware that are not permitted by the
iMX6 manuals for the data format being produced by the sensor?


Anyway, no the user is not allowed to configure a path that is not
allowed by the hardware, such as attempting to pass raw bayer through
an Image Converter path.

I guess you are simply commenting that for users of bayer sensors, the
other pipelines can be "confusing". But I trust you're not saying those
other pipelines should therefore not be present, which would be a
completely nutty argument.

Steve

Re: [PATCH 03/19] alpha: marvel: make use of raw_spinlock variants

[kbuild test robot]

Hi Julia,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.11-rc1 next-20170309]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Julia-Cartwright/fixup-usage-of-non-raw-spinlocks-in-irqchips/20170312-015922
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git 
for-next
config: alpha-alldefconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/mmzone.h:7:0,
from include/linux/gfp.h:5,
from include/linux/mm.h:9,
from arch/alpha/include/asm/io.h:7,
from arch/alpha/kernel/core_marvel.c:8:
   arch/alpha/kernel/core_marvel.c: In function 'alloc_io7':
>> arch/alpha/kernel/core_marvel.c:121:17: error: passing argument 1 of 
>> 'spinlock_check' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
 spin_lock_init(>irq_lock);
^
   include/linux/spinlock.h:293:17: note: in definition of macro 
'spin_lock_init'
 spinlock_check(_lock);\
^
   include/linux/spinlock.h:286:40: note: expected 'spinlock_t * {aka struct 
spinlock *}' but argument is of type 'raw_spinlock_t * {aka struct raw_spinlock 
*}'
static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock)
   ^~
   In file included from include/linux/mmzone.h:7:0,
from include/linux/gfp.h:5,
from include/linux/mm.h:9,
from arch/alpha/include/asm/io.h:7,
from arch/alpha/kernel/core_marvel.c:8:
>> include/linux/spinlock.h:294:29: error: 'raw_spinlock_t {aka struct 
>> raw_spinlock}' has no member named 'rlock'; did you mean 'raw_lock'?
 raw_spin_lock_init(&(_lock)->rlock);  \
^
   include/linux/spinlock.h:104:9: note: in definition of macro 
'raw_spin_lock_init'
 do { *(lock) = __RAW_SPIN_LOCK_UNLOCKED(lock); } while (0)
^~~~
>> arch/alpha/kernel/core_marvel.c:121:2: note: in expansion of macro 
>> 'spin_lock_init'
 spin_lock_init(>irq_lock);
 ^~
   cc1: some warnings being treated as errors

vim +294 include/linux/spinlock.h

c2f21ce2 Thomas Gleixner 2009-12-02  280  #endif
c2f21ce2 Thomas Gleixner 2009-12-02  281  
c2f21ce2 Thomas Gleixner 2009-12-02  282  /*
c2f21ce2 Thomas Gleixner 2009-12-02  283   * Map the spin_lock functions to the 
raw variants for PREEMPT_RT=n
c2f21ce2 Thomas Gleixner 2009-12-02  284   */
c2f21ce2 Thomas Gleixner 2009-12-02  285  
3490565b Denys Vlasenko  2015-07-13 @286  static __always_inline raw_spinlock_t 
*spinlock_check(spinlock_t *lock)
c2f21ce2 Thomas Gleixner 2009-12-02  287  {
c2f21ce2 Thomas Gleixner 2009-12-02  288return >rlock;
c2f21ce2 Thomas Gleixner 2009-12-02  289  }
c2f21ce2 Thomas Gleixner 2009-12-02  290  
c2f21ce2 Thomas Gleixner 2009-12-02  291  #define spin_lock_init(_lock) 
\
c2f21ce2 Thomas Gleixner 2009-12-02  292  do {  
\
c2f21ce2 Thomas Gleixner 2009-12-02  293spinlock_check(_lock);  
\
c2f21ce2 Thomas Gleixner 2009-12-02 @294
raw_spin_lock_init(&(_lock)->rlock);\
c2f21ce2 Thomas Gleixner 2009-12-02  295  } while (0)
c2f21ce2 Thomas Gleixner 2009-12-02  296  
3490565b Denys Vlasenko  2015-07-13  297  static __always_inline void 
spin_lock(spinlock_t *lock)

:: The code at line 294 was first introduced by commit
:: c2f21ce2e31286a0a32f8da0a7856e9ca1122ef3 locking: Implement new 
raw_spinlock

:: TO: Thomas Gleixner 
:: CC: Thomas Gleixner 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 1/3] Staging: ks7010: ks_*: Use preferred 'u8' kernel type over 'uint8_t'

[Tobin C. Harding]

On Sun, Mar 12, 2017 at 03:29:59AM +0100, Shiva Kerdel wrote:
> Fix prefer kernel type 'u8' over 'uint8_t' checks.
> 
> Signed-off-by: Shiva Kerdel 
> ---
>  drivers/staging/ks7010/ks_hostif.c   |   4 +-
>  drivers/staging/ks7010/ks_hostif.h   | 114 
> +--
>  drivers/staging/ks7010/ks_wlan_net.c |   2 +-
>  3 files changed, 60 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c 
> b/drivers/staging/ks7010/ks_hostif.c
> index 6522bc3d30d5..0d6bbe61131c 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -2384,8 +2384,8 @@ void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
>   struct pmk_cache_t {
>   uint16_t size;
>   struct {
> - uint8_t bssid[ETH_ALEN];
> - uint8_t pmkid[IW_PMKID_LEN];
> + u8 bssid[ETH_ALEN];
> + u8 pmkid[IW_PMKID_LEN];
>   } __packed list[PMK_LIST_MAX];
>   } __packed pmkcache;
>   struct pmk_t *pmk;
> diff --git a/drivers/staging/ks7010/ks_hostif.h 
> b/drivers/staging/ks7010/ks_hostif.h
> index 35bfadf4665d..be8ded44c1ac 100644
> --- a/drivers/staging/ks7010/ks_hostif.h
> +++ b/drivers/staging/ks7010/ks_hostif.h
> @@ -72,7 +72,7 @@ struct hostif_data_request_t {
>  #define TYPE_DATA 0x
>  #define TYPE_AUTH 0x0001
>   uint16_t reserved;
> - uint8_t data[0];
> + u8 data[0];
>  } __packed;
>  
>  struct hostif_data_indication_t {
> @@ -83,14 +83,14 @@ struct hostif_data_indication_t {
>  #define TYPE_GMK1 0x0002
>  #define TYPE_GMK2 0x0003
>   uint16_t reserved;
> - uint8_t data[0];
> + u8 data[0];
>  } __packed;
>  
>  #define CHANNEL_LIST_MAX_SIZE 14
>  struct channel_list_t {
> - uint8_t size;
> - uint8_t body[CHANNEL_LIST_MAX_SIZE];
> - uint8_t pad;
> + u8 size;
> + u8 body[CHANNEL_LIST_MAX_SIZE];
> + u8 pad;
>  } __packed;
>  
>  /* MIB Attribute */
> @@ -154,7 +154,7 @@ struct hostif_mib_value_t {
>  #define MIB_VALUE_TYPE_BOOL 2
>  #define MIB_VALUE_TYPE_COUNT32  3
>  #define MIB_VALUE_TYPE_OSTRING  4
> - uint8_t body[0];
> + u8 body[0];
>  } __packed;
>  
>  struct hostif_mib_get_confirm_t {
> @@ -227,32 +227,32 @@ struct hostif_start_confirm_t {
>  
>  #define SSID_MAX_SIZE 32
>  struct ssid_t {
> - uint8_t size;
> - uint8_t body[SSID_MAX_SIZE];
> - uint8_t ssid_pad;
> + u8 size;
> + u8 body[SSID_MAX_SIZE];
> + u8 ssid_pad;
>  } __packed;
>  
>  #define RATE_SET_MAX_SIZE 16
>  struct rate_set8_t {
> - uint8_t size;
> - uint8_t body[8];
> - uint8_t rate_pad;
> + u8 size;
> + u8 body[8];
> + u8 rate_pad;
>  } __packed;
>  
>  struct FhParms_t {
>   uint16_t dwellTime;
> - uint8_t hopSet;
> - uint8_t hopPattern;
> - uint8_t hopIndex;
> + u8 hopSet;
> + u8 hopPattern;
> + u8 hopIndex;
>  } __packed;
>  
>  struct DsParms_t {
> - uint8_t channel;
> + u8 channel;
>  } __packed;
>  
>  struct CfParms_t {
> - uint8_t count;
> - uint8_t period;
> + u8 count;
> + u8 period;
>   uint16_t maxDuration;
>   uint16_t durRemaining;
>  } __packed;
> @@ -262,27 +262,27 @@ struct IbssParms_t {
>  } __packed;
>  
>  struct rsn_t {
> - uint8_t size;
> + u8 size;
>  #define RSN_BODY_SIZE 64
> - uint8_t body[RSN_BODY_SIZE];
> + u8 body[RSN_BODY_SIZE];
>  } __packed;
>  
>  struct ErpParams_t {
> - uint8_t erp_info;
> + u8 erp_info;
>  } __packed;
>  
>  struct rate_set16_t {
> - uint8_t size;
> - uint8_t body[16];
> - uint8_t rate_pad;
> + u8 size;
> + u8 body[16];
> + u8 rate_pad;
>  } __packed;
>  
>  struct ap_info_t {
> - uint8_t bssid[6];   /* +00 */
> - uint8_t rssi;   /* +06 */
> - uint8_t sq; /* +07 */
> - uint8_t noise;  /* +08 */
> - uint8_t pad0;   /* +09 */
> + u8 bssid[6];/* +00 */
> + u8 rssi;/* +06 */
> + u8 sq;  /* +07 */
> + u8 noise;   /* +08 */
> + u8 pad0;/* +09 */
>   uint16_t beacon_period; /* +10 */
>   uint16_t capability;/* +12 */
>  #define BSS_CAP_ESS BIT(0)
> @@ -295,21 +295,21 @@ struct ap_info_t {
>  #define BSS_CAP_CHANNEL_AGILITY BIT(7)
>  #define BSS_CAP_SHORT_SLOT_TIME BIT(10)
>  #define BSS_CAP_DSSS_OFDM   BIT(13)
> - uint8_t frame_type; /* +14 */
> - uint8_t ch_info;/* +15 */
> + u8 frame_type;  /* +14 */
> + u8 ch_info; /* +15 */
>  #define FRAME_TYPE_BEACON0x80
>  #define FRAME_TYPE_PROBE_RESP0x50
>   uint16_t body_size; /* +16 */
> - uint8_t body[1024]; /* +18 */
> + u8 body[1024];  /* +18 */
>   /* +1032 */
>  } __packed;
>  
>  struct link_ap_info_t {
> - uint8_t bssid[6];   /* +00 */
> - uint8_t rssi;   /* +06 */
> - uint8_t sq; /* +07 */
> - uint8_t noise;  /* +08 */
> - uint8_t pad0;   /* +09 */

Re: [PATCH 1/3] Staging: ks7010: ks_*: Use preferred 'u8' kernel type over 'uint8_t'

[Tobin C. Harding]

On Sun, Mar 12, 2017 at 03:29:59AM +0100, Shiva Kerdel wrote:
> Fix prefer kernel type 'u8' over 'uint8_t' checks.
> 
> Signed-off-by: Shiva Kerdel 
> ---
>  drivers/staging/ks7010/ks_hostif.c   |   4 +-
>  drivers/staging/ks7010/ks_hostif.h   | 114 
> +--
>  drivers/staging/ks7010/ks_wlan_net.c |   2 +-
>  3 files changed, 60 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c 
> b/drivers/staging/ks7010/ks_hostif.c
> index 6522bc3d30d5..0d6bbe61131c 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -2384,8 +2384,8 @@ void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
>   struct pmk_cache_t {
>   uint16_t size;
>   struct {
> - uint8_t bssid[ETH_ALEN];
> - uint8_t pmkid[IW_PMKID_LEN];
> + u8 bssid[ETH_ALEN];
> + u8 pmkid[IW_PMKID_LEN];
>   } __packed list[PMK_LIST_MAX];
>   } __packed pmkcache;
>   struct pmk_t *pmk;
> diff --git a/drivers/staging/ks7010/ks_hostif.h 
> b/drivers/staging/ks7010/ks_hostif.h
> index 35bfadf4665d..be8ded44c1ac 100644
> --- a/drivers/staging/ks7010/ks_hostif.h
> +++ b/drivers/staging/ks7010/ks_hostif.h
> @@ -72,7 +72,7 @@ struct hostif_data_request_t {
>  #define TYPE_DATA 0x
>  #define TYPE_AUTH 0x0001
>   uint16_t reserved;
> - uint8_t data[0];
> + u8 data[0];
>  } __packed;
>  
>  struct hostif_data_indication_t {
> @@ -83,14 +83,14 @@ struct hostif_data_indication_t {
>  #define TYPE_GMK1 0x0002
>  #define TYPE_GMK2 0x0003
>   uint16_t reserved;
> - uint8_t data[0];
> + u8 data[0];
>  } __packed;
>  
>  #define CHANNEL_LIST_MAX_SIZE 14
>  struct channel_list_t {
> - uint8_t size;
> - uint8_t body[CHANNEL_LIST_MAX_SIZE];
> - uint8_t pad;
> + u8 size;
> + u8 body[CHANNEL_LIST_MAX_SIZE];
> + u8 pad;
>  } __packed;
>  
>  /* MIB Attribute */
> @@ -154,7 +154,7 @@ struct hostif_mib_value_t {
>  #define MIB_VALUE_TYPE_BOOL 2
>  #define MIB_VALUE_TYPE_COUNT32  3
>  #define MIB_VALUE_TYPE_OSTRING  4
> - uint8_t body[0];
> + u8 body[0];
>  } __packed;
>  
>  struct hostif_mib_get_confirm_t {
> @@ -227,32 +227,32 @@ struct hostif_start_confirm_t {
>  
>  #define SSID_MAX_SIZE 32
>  struct ssid_t {
> - uint8_t size;
> - uint8_t body[SSID_MAX_SIZE];
> - uint8_t ssid_pad;
> + u8 size;
> + u8 body[SSID_MAX_SIZE];
> + u8 ssid_pad;
>  } __packed;
>  
>  #define RATE_SET_MAX_SIZE 16
>  struct rate_set8_t {
> - uint8_t size;
> - uint8_t body[8];
> - uint8_t rate_pad;
> + u8 size;
> + u8 body[8];
> + u8 rate_pad;
>  } __packed;
>  
>  struct FhParms_t {
>   uint16_t dwellTime;
> - uint8_t hopSet;
> - uint8_t hopPattern;
> - uint8_t hopIndex;
> + u8 hopSet;
> + u8 hopPattern;
> + u8 hopIndex;
>  } __packed;
>  
>  struct DsParms_t {
> - uint8_t channel;
> + u8 channel;
>  } __packed;
>  
>  struct CfParms_t {
> - uint8_t count;
> - uint8_t period;
> + u8 count;
> + u8 period;
>   uint16_t maxDuration;
>   uint16_t durRemaining;
>  } __packed;
> @@ -262,27 +262,27 @@ struct IbssParms_t {
>  } __packed;
>  
>  struct rsn_t {
> - uint8_t size;
> + u8 size;
>  #define RSN_BODY_SIZE 64
> - uint8_t body[RSN_BODY_SIZE];
> + u8 body[RSN_BODY_SIZE];
>  } __packed;
>  
>  struct ErpParams_t {
> - uint8_t erp_info;
> + u8 erp_info;
>  } __packed;
>  
>  struct rate_set16_t {
> - uint8_t size;
> - uint8_t body[16];
> - uint8_t rate_pad;
> + u8 size;
> + u8 body[16];
> + u8 rate_pad;
>  } __packed;
>  
>  struct ap_info_t {
> - uint8_t bssid[6];   /* +00 */
> - uint8_t rssi;   /* +06 */
> - uint8_t sq; /* +07 */
> - uint8_t noise;  /* +08 */
> - uint8_t pad0;   /* +09 */
> + u8 bssid[6];/* +00 */
> + u8 rssi;/* +06 */
> + u8 sq;  /* +07 */
> + u8 noise;   /* +08 */
> + u8 pad0;/* +09 */
>   uint16_t beacon_period; /* +10 */
>   uint16_t capability;/* +12 */
>  #define BSS_CAP_ESS BIT(0)
> @@ -295,21 +295,21 @@ struct ap_info_t {
>  #define BSS_CAP_CHANNEL_AGILITY BIT(7)
>  #define BSS_CAP_SHORT_SLOT_TIME BIT(10)
>  #define BSS_CAP_DSSS_OFDM   BIT(13)
> - uint8_t frame_type; /* +14 */
> - uint8_t ch_info;/* +15 */
> + u8 frame_type;  /* +14 */
> + u8 ch_info; /* +15 */
>  #define FRAME_TYPE_BEACON0x80
>  #define FRAME_TYPE_PROBE_RESP0x50
>   uint16_t body_size; /* +16 */
> - uint8_t body[1024]; /* +18 */
> + u8 body[1024];  /* +18 */
>   /* +1032 */
>  } __packed;
>  
>  struct link_ap_info_t {
> - uint8_t bssid[6];   /* +00 */
> - uint8_t rssi;   /* +06 */
> - uint8_t sq; /* +07 */
> - uint8_t noise;  /* +08 */
> - uint8_t pad0;   /* +09 */
> + u8 

Re: [PATCH 03/19] alpha: marvel: make use of raw_spinlock variants

[kbuild test robot]

Hi Julia,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.11-rc1 next-20170309]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Julia-Cartwright/fixup-usage-of-non-raw-spinlocks-in-irqchips/20170312-015922
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git 
for-next
config: alpha-alldefconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/mmzone.h:7:0,
from include/linux/gfp.h:5,
from include/linux/mm.h:9,
from arch/alpha/include/asm/io.h:7,
from arch/alpha/kernel/core_marvel.c:8:
   arch/alpha/kernel/core_marvel.c: In function 'alloc_io7':
>> arch/alpha/kernel/core_marvel.c:121:17: error: passing argument 1 of 
>> 'spinlock_check' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
 spin_lock_init(>irq_lock);
^
   include/linux/spinlock.h:293:17: note: in definition of macro 
'spin_lock_init'
 spinlock_check(_lock);\
^
   include/linux/spinlock.h:286:40: note: expected 'spinlock_t * {aka struct 
spinlock *}' but argument is of type 'raw_spinlock_t * {aka struct raw_spinlock 
*}'
static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock)
   ^~
   In file included from include/linux/mmzone.h:7:0,
from include/linux/gfp.h:5,
from include/linux/mm.h:9,
from arch/alpha/include/asm/io.h:7,
from arch/alpha/kernel/core_marvel.c:8:
>> include/linux/spinlock.h:294:29: error: 'raw_spinlock_t {aka struct 
>> raw_spinlock}' has no member named 'rlock'; did you mean 'raw_lock'?
 raw_spin_lock_init(&(_lock)->rlock);  \
^
   include/linux/spinlock.h:104:9: note: in definition of macro 
'raw_spin_lock_init'
 do { *(lock) = __RAW_SPIN_LOCK_UNLOCKED(lock); } while (0)
^~~~
>> arch/alpha/kernel/core_marvel.c:121:2: note: in expansion of macro 
>> 'spin_lock_init'
 spin_lock_init(>irq_lock);
 ^~
   cc1: some warnings being treated as errors

vim +294 include/linux/spinlock.h

c2f21ce2 Thomas Gleixner 2009-12-02  280  #endif
c2f21ce2 Thomas Gleixner 2009-12-02  281  
c2f21ce2 Thomas Gleixner 2009-12-02  282  /*
c2f21ce2 Thomas Gleixner 2009-12-02  283   * Map the spin_lock functions to the 
raw variants for PREEMPT_RT=n
c2f21ce2 Thomas Gleixner 2009-12-02  284   */
c2f21ce2 Thomas Gleixner 2009-12-02  285  
3490565b Denys Vlasenko  2015-07-13 @286  static __always_inline raw_spinlock_t 
*spinlock_check(spinlock_t *lock)
c2f21ce2 Thomas Gleixner 2009-12-02  287  {
c2f21ce2 Thomas Gleixner 2009-12-02  288return >rlock;
c2f21ce2 Thomas Gleixner 2009-12-02  289  }
c2f21ce2 Thomas Gleixner 2009-12-02  290  
c2f21ce2 Thomas Gleixner 2009-12-02  291  #define spin_lock_init(_lock) 
\
c2f21ce2 Thomas Gleixner 2009-12-02  292  do {  
\
c2f21ce2 Thomas Gleixner 2009-12-02  293spinlock_check(_lock);  
\
c2f21ce2 Thomas Gleixner 2009-12-02 @294
raw_spin_lock_init(&(_lock)->rlock);\
c2f21ce2 Thomas Gleixner 2009-12-02  295  } while (0)
c2f21ce2 Thomas Gleixner 2009-12-02  296  
3490565b Denys Vlasenko  2015-07-13  297  static __always_inline void 
spin_lock(spinlock_t *lock)

:: The code at line 294 was first introduced by commit
:: c2f21ce2e31286a0a32f8da0a7856e9ca1122ef3 locking: Implement new 
raw_spinlock

:: TO: Thomas Gleixner 
:: CC: Thomas Gleixner 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH] staging: sm750fb: Improved coding style

[Arushi Singhal]

New variables are added to make the code more readable.
Also fixed the checkpatch issue: "Alignment should match open
parenthesis".

Signed-off-by: Arushi Singhal 
---
 drivers/staging/sm750fb/ddk750_mode.c | 112 --
 1 file changed, 53 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
b/drivers/staging/sm750fb/ddk750_mode.c
index 45af806c8d55..7fd3c3f20c01 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -28,9 +28,9 @@ static unsigned long 
displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
poke32(CRT_AUTO_CENTERING_TL, 0);
 
poke32(CRT_AUTO_CENTERING_BR,
-   (((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
-   CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
-   ((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
+  (((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
+   CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
+  ((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
 
/*
 * Assume common fields in dispControl have been properly set before
@@ -72,43 +72,39 @@ static unsigned long 
displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
 }
 
 /* only timing related registers will be  programed */
-static int programModeRegisters(mode_parameter_t *pModeParam,
-   struct pll_value *pll)
+static int programModeRegisters(mode_parameter_t *pModeParam, struct pll_value 
*pll)
 {
int ret = 0;
int cnt = 0;
unsigned int tmp, reg;
+   unsigned int cht = CRT_HORIZONTAL_TOTAL;
+   unsigned int cvt = CRT_VERTICAL_TOTAL;
+   unsigned int chs = CRT_HORIZONTAL_SYNC;
+   unsigned int cvs = CRT_VERTICAL_SYNC;
+   unsigned int chssm = CRT_HORIZONTAL_SYNC_START_MASK;
+   unsigned int cvssm = CRT_VERTICAL_SYNC_START_MASK;
+   unsigned int chtdem = CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK;
+   unsigned int cvtdem = CRT_VERTICAL_TOTAL_DISPLAY_END_MASK;
+   unsigned int chttm = CRT_HORIZONTAL_TOTAL_TOTAL_MASK;
+   unsigned int cvttm = CRT_VERTICAL_TOTAL_TOTAL_MASK;
+   unsigned int chswm = CRT_HORIZONTAL_SYNC_WIDTH_MASK;
+   unsigned int cvshm = CRT_VERTICAL_SYNC_HEIGHT_MASK;
+   unsigned int phde = pModeParam->horizontal_display_end - 1;
+   unsigned int pvde = pModeParam->vertical_display_end - 1;
+   unsigned int phss = pModeParam->horizontal_sync_start - 1;
+   unsigned int pvss = pModeParam->vertical_sync_start - 1;
 
if (pll->clockType == SECONDARY_PLL) {
/* programe secondary pixel clock */
poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
-   poke32(CRT_HORIZONTAL_TOTAL,
-   (((pModeParam->horizontal_total - 1) <<
-   CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
-   CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
-   ((pModeParam->horizontal_display_end - 1) &
-   CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
-
-   poke32(CRT_HORIZONTAL_SYNC,
-   ((pModeParam->horizontal_sync_width <<
-   CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
-   CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
-   ((pModeParam->horizontal_sync_start - 1) &
-   CRT_HORIZONTAL_SYNC_START_MASK));
-
-   poke32(CRT_VERTICAL_TOTAL,
-   (((pModeParam->vertical_total - 1) <<
-   CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
-   CRT_VERTICAL_TOTAL_TOTAL_MASK) |
-   ((pModeParam->vertical_display_end - 1) &
-   CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
-
-   poke32(CRT_VERTICAL_SYNC,
-   ((pModeParam->vertical_sync_height <<
-   CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
-   CRT_VERTICAL_SYNC_HEIGHT_MASK) |
-   ((pModeParam->vertical_sync_start - 1) &
-   CRT_VERTICAL_SYNC_START_MASK));
+
+   poke32(cht, (((pModeParam->horizontal_total - 1) << 
CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) & chttm) | (phde & chtdem));
+
+   poke32(chs, ((pModeParam->horizontal_sync_width << 
CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) & chswm) | (phss & chssm));
+
+   poke32(cvt, (((pModeParam->vertical_total - 1) << 
CRT_VERTICAL_TOTAL_TOTAL_SHIFT) & cvttm) | (pvde & cvtdem));
+
+   poke32(cvs, ((pModeParam->vertical_sync_height << 
CRT_VERTICAL_SYNC_HEIGHT_SHIFT) & cvshm) | (pvss & cvssm));
 
tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE;
if (pModeParam->vertical_sync_polarity)
@@ -129,36 +125,34 @@ static int programModeRegisters(mode_parameter_t 

[PATCH] staging: sm750fb: Improved coding style

[Arushi Singhal]

New variables are added to make the code more readable.
Also fixed the checkpatch issue: "Alignment should match open
parenthesis".

Signed-off-by: Arushi Singhal 
---
 drivers/staging/sm750fb/ddk750_mode.c | 112 --
 1 file changed, 53 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
b/drivers/staging/sm750fb/ddk750_mode.c
index 45af806c8d55..7fd3c3f20c01 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -28,9 +28,9 @@ static unsigned long 
displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
poke32(CRT_AUTO_CENTERING_TL, 0);
 
poke32(CRT_AUTO_CENTERING_BR,
-   (((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
-   CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
-   ((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
+  (((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
+   CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
+  ((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
 
/*
 * Assume common fields in dispControl have been properly set before
@@ -72,43 +72,39 @@ static unsigned long 
displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
 }
 
 /* only timing related registers will be  programed */
-static int programModeRegisters(mode_parameter_t *pModeParam,
-   struct pll_value *pll)
+static int programModeRegisters(mode_parameter_t *pModeParam, struct pll_value 
*pll)
 {
int ret = 0;
int cnt = 0;
unsigned int tmp, reg;
+   unsigned int cht = CRT_HORIZONTAL_TOTAL;
+   unsigned int cvt = CRT_VERTICAL_TOTAL;
+   unsigned int chs = CRT_HORIZONTAL_SYNC;
+   unsigned int cvs = CRT_VERTICAL_SYNC;
+   unsigned int chssm = CRT_HORIZONTAL_SYNC_START_MASK;
+   unsigned int cvssm = CRT_VERTICAL_SYNC_START_MASK;
+   unsigned int chtdem = CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK;
+   unsigned int cvtdem = CRT_VERTICAL_TOTAL_DISPLAY_END_MASK;
+   unsigned int chttm = CRT_HORIZONTAL_TOTAL_TOTAL_MASK;
+   unsigned int cvttm = CRT_VERTICAL_TOTAL_TOTAL_MASK;
+   unsigned int chswm = CRT_HORIZONTAL_SYNC_WIDTH_MASK;
+   unsigned int cvshm = CRT_VERTICAL_SYNC_HEIGHT_MASK;
+   unsigned int phde = pModeParam->horizontal_display_end - 1;
+   unsigned int pvde = pModeParam->vertical_display_end - 1;
+   unsigned int phss = pModeParam->horizontal_sync_start - 1;
+   unsigned int pvss = pModeParam->vertical_sync_start - 1;
 
if (pll->clockType == SECONDARY_PLL) {
/* programe secondary pixel clock */
poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
-   poke32(CRT_HORIZONTAL_TOTAL,
-   (((pModeParam->horizontal_total - 1) <<
-   CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
-   CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
-   ((pModeParam->horizontal_display_end - 1) &
-   CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
-
-   poke32(CRT_HORIZONTAL_SYNC,
-   ((pModeParam->horizontal_sync_width <<
-   CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
-   CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
-   ((pModeParam->horizontal_sync_start - 1) &
-   CRT_HORIZONTAL_SYNC_START_MASK));
-
-   poke32(CRT_VERTICAL_TOTAL,
-   (((pModeParam->vertical_total - 1) <<
-   CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
-   CRT_VERTICAL_TOTAL_TOTAL_MASK) |
-   ((pModeParam->vertical_display_end - 1) &
-   CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
-
-   poke32(CRT_VERTICAL_SYNC,
-   ((pModeParam->vertical_sync_height <<
-   CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
-   CRT_VERTICAL_SYNC_HEIGHT_MASK) |
-   ((pModeParam->vertical_sync_start - 1) &
-   CRT_VERTICAL_SYNC_START_MASK));
+
+   poke32(cht, (((pModeParam->horizontal_total - 1) << 
CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) & chttm) | (phde & chtdem));
+
+   poke32(chs, ((pModeParam->horizontal_sync_width << 
CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) & chswm) | (phss & chssm));
+
+   poke32(cvt, (((pModeParam->vertical_total - 1) << 
CRT_VERTICAL_TOTAL_TOTAL_SHIFT) & cvttm) | (pvde & cvtdem));
+
+   poke32(cvs, ((pModeParam->vertical_sync_height << 
CRT_VERTICAL_SYNC_HEIGHT_SHIFT) & cvshm) | (pvss & cvssm));
 
tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE;
if (pModeParam->vertical_sync_polarity)
@@ -129,36 +125,34 @@ static int programModeRegisters(mode_parameter_t 
*pModeParam,
 
} else if 

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Al Viro]

On Sat, Mar 11, 2017 at 06:16:55PM -0800, Eric Biggers wrote:
> Hi Al,
> 
> On Sun, Mar 12, 2017 at 01:24:15AM +, Al Viro wrote:
> > On Sat, Mar 11, 2017 at 01:45:55PM -0800, Eric Biggers wrote:
> > > From: Eric Biggers 
> > > 
> > > I found that statx() was significantly slower than stat().  As a
> > > microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
> > > file to the same with statx() passed a NULL path:
> > 
> > Umm...
> > 
> 
> Well, it's a silly benchmark, but stat performance is important, and usually
> things are cached already so most of the time is just overhead --- which this
> measures.  And since nothing actually uses statx() yet, you can't do a 
> benchmark
> just by running some command like 'git status' or whatever.

Oh, I agree that multiple __put_user() are wrong; I also agree that bulk copy is
the right approach (when we get the unsafe stuff right, we can revisit that, but
I suspect that on quite a few architectures a bulk copy will still give better
time, no matter what).

> If padding is a concern at all (AFAICS it's not actually an issue now with
> struct statx, but people tend to have different opinions on how careful they
> want to be with padding), then I think we'll just have to start by memsetting
> the whole struct to 0.

My point is simply that it's worth a comment in that code.

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Al Viro]

On Sat, Mar 11, 2017 at 06:16:55PM -0800, Eric Biggers wrote:
> Hi Al,
> 
> On Sun, Mar 12, 2017 at 01:24:15AM +, Al Viro wrote:
> > On Sat, Mar 11, 2017 at 01:45:55PM -0800, Eric Biggers wrote:
> > > From: Eric Biggers 
> > > 
> > > I found that statx() was significantly slower than stat().  As a
> > > microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
> > > file to the same with statx() passed a NULL path:
> > 
> > Umm...
> > 
> 
> Well, it's a silly benchmark, but stat performance is important, and usually
> things are cached already so most of the time is just overhead --- which this
> measures.  And since nothing actually uses statx() yet, you can't do a 
> benchmark
> just by running some command like 'git status' or whatever.

Oh, I agree that multiple __put_user() are wrong; I also agree that bulk copy is
the right approach (when we get the unsafe stuff right, we can revisit that, but
I suspect that on quite a few architectures a bulk copy will still give better
time, no matter what).

> If padding is a concern at all (AFAICS it's not actually an issue now with
> struct statx, but people tend to have different opinions on how careful they
> want to be with padding), then I think we'll just have to start by memsetting
> the whole struct to 0.

My point is simply that it's worth a comment in that code.

Re: [PATCH net-next v2 00/17] net: dsa: mv88e6xxx: rework ATU support

[Florian Fainelli]

Le 03/11/17 à 13:12, Vivien Didelot a écrit :
> The purpose of this patch series is to rework the code related to the
> Address Translation Unit (ATU), and bring support for it to the 88E6390
> family of switch chips.
> 
> All Global (1) ATU related code have been reworked and moved to its own
> file. Some port related bits used for ATU configuration (such as the
> Learn2All and MessagePort feature) have also been taken care of.
> 
> The ports' mode and egress flooding mode have been refactored to fix the
> egress of frames with unknown unicast or multicast destination address,
> and write all these bits regardless the port mode (Normal, DSA, etc.)
> 
> Finally remove the eth_addr_greater which was only used by mv88e6xxx.

Just some general note on the way the driver seems to be going there
seems to be a multiplication of new ops being introduced, and most of
them seem to default to the mv88e6xxx (generic) ones. Should you do
something like:

if (!ops->foo)
ops->foo = mv88e6xxx_foo

Such that you reduce the possibility for a specific switch model to lack
such an operation?

> 
> Changes in v2:
>   - add Reviewed-by tags
>   - split mv88e6xxx_g1_atu_set_age_time and mv88e6xxx_atu_setup addition
>   - remove DSA_TAG_PROTO_TRAILER check
>   - split Message Port and Learn2All addition
>   - remove unused MV88E6XXX_FLAG_G1_ATU_FID flag
>   - add dsa_is_normal_port helper
> 
> Vivien Didelot (17):
>   net: dsa: mv88e6xxx: add port mask helper
>   net: dsa: mv88e6xxx: move ATU ageing time setter
>   net: dsa: mv88e6xxx: add ATU setup helper
>   net: dsa: mv88e6xxx: setup message ports
>   net: dsa: mv88e6xxx: enable ATU Learn2All
>   net: dsa: mv88e6xxx: rework ATU Load/Purge
>   net: dsa: mv88e6xxx: rework ATU GetNext
>   net: dsa: mv88e6xxx: rework ATU Flush
>   net: dsa: mv88e6xxx: rework ATU Remove
>   net: dsa: mv88e6xxx: rename new FID helper
>   net: dsa: mv88e6xxx: rename the port vector member
>   net: dsa: add dsa_is_normal_port helper
>   net: dsa: mv88e6xxx: rework port mode setup
>   net: dsa: mv88e6xxx: fix port egress flooding mode
>   net: dsa: mv88e6xxx: add port ATU learn limit op
>   net: dsa: mv88e6xxx: add port priority override op
>   etherdevice: remove unused eth_addr_greater
> 
>  drivers/net/dsa/mv88e6xxx/Makefile  |   1 +
>  drivers/net/dsa/mv88e6xxx/chip.c| 667 
> +++-
>  drivers/net/dsa/mv88e6xxx/global1.h |  11 +
>  drivers/net/dsa/mv88e6xxx/global1_atu.c | 300 ++
>  drivers/net/dsa/mv88e6xxx/mv88e6xxx.h   |  44 ++-
>  drivers/net/dsa/mv88e6xxx/port.c|  78 +++-
>  drivers/net/dsa/mv88e6xxx/port.h|  16 +-
>  include/linux/etherdevice.h |  15 -
>  include/net/dsa.h   |   5 +
>  9 files changed, 638 insertions(+), 499 deletions(-)
>  create mode 100644 drivers/net/dsa/mv88e6xxx/global1_atu.c
> 


-- 
Florian

Re: [PATCH net-next v2 00/17] net: dsa: mv88e6xxx: rework ATU support

[Florian Fainelli]

Le 03/11/17 à 13:12, Vivien Didelot a écrit :
> The purpose of this patch series is to rework the code related to the
> Address Translation Unit (ATU), and bring support for it to the 88E6390
> family of switch chips.
> 
> All Global (1) ATU related code have been reworked and moved to its own
> file. Some port related bits used for ATU configuration (such as the
> Learn2All and MessagePort feature) have also been taken care of.
> 
> The ports' mode and egress flooding mode have been refactored to fix the
> egress of frames with unknown unicast or multicast destination address,
> and write all these bits regardless the port mode (Normal, DSA, etc.)
> 
> Finally remove the eth_addr_greater which was only used by mv88e6xxx.

Just some general note on the way the driver seems to be going there
seems to be a multiplication of new ops being introduced, and most of
them seem to default to the mv88e6xxx (generic) ones. Should you do
something like:

if (!ops->foo)
ops->foo = mv88e6xxx_foo

Such that you reduce the possibility for a specific switch model to lack
such an operation?

> 
> Changes in v2:
>   - add Reviewed-by tags
>   - split mv88e6xxx_g1_atu_set_age_time and mv88e6xxx_atu_setup addition
>   - remove DSA_TAG_PROTO_TRAILER check
>   - split Message Port and Learn2All addition
>   - remove unused MV88E6XXX_FLAG_G1_ATU_FID flag
>   - add dsa_is_normal_port helper
> 
> Vivien Didelot (17):
>   net: dsa: mv88e6xxx: add port mask helper
>   net: dsa: mv88e6xxx: move ATU ageing time setter
>   net: dsa: mv88e6xxx: add ATU setup helper
>   net: dsa: mv88e6xxx: setup message ports
>   net: dsa: mv88e6xxx: enable ATU Learn2All
>   net: dsa: mv88e6xxx: rework ATU Load/Purge
>   net: dsa: mv88e6xxx: rework ATU GetNext
>   net: dsa: mv88e6xxx: rework ATU Flush
>   net: dsa: mv88e6xxx: rework ATU Remove
>   net: dsa: mv88e6xxx: rename new FID helper
>   net: dsa: mv88e6xxx: rename the port vector member
>   net: dsa: add dsa_is_normal_port helper
>   net: dsa: mv88e6xxx: rework port mode setup
>   net: dsa: mv88e6xxx: fix port egress flooding mode
>   net: dsa: mv88e6xxx: add port ATU learn limit op
>   net: dsa: mv88e6xxx: add port priority override op
>   etherdevice: remove unused eth_addr_greater
> 
>  drivers/net/dsa/mv88e6xxx/Makefile  |   1 +
>  drivers/net/dsa/mv88e6xxx/chip.c| 667 
> +++-
>  drivers/net/dsa/mv88e6xxx/global1.h |  11 +
>  drivers/net/dsa/mv88e6xxx/global1_atu.c | 300 ++
>  drivers/net/dsa/mv88e6xxx/mv88e6xxx.h   |  44 ++-
>  drivers/net/dsa/mv88e6xxx/port.c|  78 +++-
>  drivers/net/dsa/mv88e6xxx/port.h|  16 +-
>  include/linux/etherdevice.h |  15 -
>  include/net/dsa.h   |   5 +
>  9 files changed, 638 insertions(+), 499 deletions(-)
>  create mode 100644 drivers/net/dsa/mv88e6xxx/global1_atu.c
> 


-- 
Florian

[PATCH v2] audit: log module name on delete_module

[Richard Guy Briggs]

When a sysadmin wishes to monitor module unloading with a syscall rule such as:
 -a always,exit -F arch=x86_64 -S delete_module -F key=mod-unload
the SYSCALL record doesn't tell us what module was requested for unloading.

Use the new KERN_MODULE auxiliary record to record it.
The SYSCALL record result code will list the return code.

See: https://github.com/linux-audit/audit-kernel/issues/37
https://github.com/linux-audit/audit-kernel/issues/7

https://github.com/linux-audit/audit-kernel/wiki/RFE-Module-Load-Record-Format

Signed-off-by: Richard Guy Briggs 
---
 kernel/module.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 5432dbe..633f6da 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -943,6 +943,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *, 
name_user,
return -EFAULT;
name[MODULE_NAME_LEN-1] = '\0';
 
+   audit_log_kern_module(name);
+
if (mutex_lock_interruptible(_mutex) != 0)
return -EINTR;
 
-- 
1.7.1

[PATCH v2] audit: log module name on delete_module

[Richard Guy Briggs]

When a sysadmin wishes to monitor module unloading with a syscall rule such as:
 -a always,exit -F arch=x86_64 -S delete_module -F key=mod-unload
the SYSCALL record doesn't tell us what module was requested for unloading.

Use the new KERN_MODULE auxiliary record to record it.
The SYSCALL record result code will list the return code.

See: https://github.com/linux-audit/audit-kernel/issues/37
https://github.com/linux-audit/audit-kernel/issues/7

https://github.com/linux-audit/audit-kernel/wiki/RFE-Module-Load-Record-Format

Signed-off-by: Richard Guy Briggs 
---
 kernel/module.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 5432dbe..633f6da 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -943,6 +943,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *, 
name_user,
return -EFAULT;
name[MODULE_NAME_LEN-1] = '\0';
 
+   audit_log_kern_module(name);
+
if (mutex_lock_interruptible(_mutex) != 0)
return -EINTR;
 
-- 
1.7.1

Re: [PATCH net-next v2 12/17] net: dsa: add dsa_is_normal_port helper

[Florian Fainelli]

Le 03/11/17 à 13:12, Vivien Didelot a écrit :
> Introduce a dsa_is_normal_port helper to check if a given port is a
> normal user port as opposed to a CPU port or DSA link.

net/dsa/dsa2.c uses the "user" terminology should we use something like
that here?

> 
> Signed-off-by: Vivien Didelot 
> ---
>  include/net/dsa.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 4e13e695f025..bf0e42c2a6f7 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -248,6 +248,11 @@ static inline bool dsa_is_dsa_port(struct dsa_switch 
> *ds, int p)
>   return !!((ds->dsa_port_mask) & (1 << p));
>  }
>  
> +static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
> +{
> + return !dsa_is_cpu_port(ds, p) && !dsa_is_dsa_port(ds, p);
> +}
> +
>  static inline bool dsa_is_port_initialized(struct dsa_switch *ds, int p)
>  {
>   return ds->enabled_port_mask & (1 << p) && ds->ports[p].netdev;
> 


-- 
Florian

Re: [PATCH net-next v2 12/17] net: dsa: add dsa_is_normal_port helper

[Florian Fainelli]

Le 03/11/17 à 13:12, Vivien Didelot a écrit :
> Introduce a dsa_is_normal_port helper to check if a given port is a
> normal user port as opposed to a CPU port or DSA link.

net/dsa/dsa2.c uses the "user" terminology should we use something like
that here?

> 
> Signed-off-by: Vivien Didelot 
> ---
>  include/net/dsa.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 4e13e695f025..bf0e42c2a6f7 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -248,6 +248,11 @@ static inline bool dsa_is_dsa_port(struct dsa_switch 
> *ds, int p)
>   return !!((ds->dsa_port_mask) & (1 << p));
>  }
>  
> +static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
> +{
> + return !dsa_is_cpu_port(ds, p) && !dsa_is_dsa_port(ds, p);
> +}
> +
>  static inline bool dsa_is_port_initialized(struct dsa_switch *ds, int p)
>  {
>   return ds->enabled_port_mask & (1 << p) && ds->ports[p].netdev;
> 


-- 
Florian

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Eric Biggers]

Hi Al,

On Sun, Mar 12, 2017 at 01:24:15AM +, Al Viro wrote:
> On Sat, Mar 11, 2017 at 01:45:55PM -0800, Eric Biggers wrote:
> > From: Eric Biggers 
> > 
> > I found that statx() was significantly slower than stat().  As a
> > microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
> > file to the same with statx() passed a NULL path:
> 
> Umm...
> 

Well, it's a silly benchmark, but stat performance is important, and usually
things are cached already so most of the time is just overhead --- which this
measures.  And since nothing actually uses statx() yet, you can't do a benchmark
just by running some command like 'git status' or whatever.

> > +   tmp.stx_dev_major = MAJOR(stat->dev);
> > +   tmp.stx_dev_minor = MINOR(stat->dev);
> > +   memset(tmp.__spare2, 0, sizeof(tmp.__spare2));
> > +
> > +   return copy_to_user(buffer, , sizeof(tmp)) ? -EFAULT : 0;
> 
> That relies upon there being no padding in the damn structure.
> It's true and probably will be true on any target, but
>   a) it's bloody well worth stating explicitly
> and
>   b) struct statx tmp = {.stx_mask = stat->result_mask};
> will get rid of those memset() you've got there by implicit
> zeroing of fields missing in partial structure initializer.
> Padding is *not* included into that, but you are relying upon
> having no padding anyway...

If padding is a concern at all (AFAICS it's not actually an issue now with
struct statx, but people tend to have different opinions on how careful they
want to be with padding), then I think we'll just have to start by memsetting
the whole struct to 0.

stat() actually does that, except that it's overridden on some architectures,
like x86, to only memset the actual reserved fields.  I had kind of assumed that
as long as we're defining a new struct that has the same layout on all
architectures, with no padding, that we'd want to take the opportunity to only
memset the actual reserved fields, to make it a bit faster.  And yes, a
designated initializer would make this look a bit nicer, though it might have to
be changed later if any future statx() extensions involve fields that are
unioned or conditionally written.

Note that another approach would be to copy the fields individually, but with
unsafe_put_user() instead of __put_user().  That would avoid the overhead of
user_access_begin()/user_access_end() which was the main performance problem.
However, the infrastructure around this doesn't seem to be ready quite yet:
there aren't good implementations of unsafe_put_user() yet, nor is there an
unsafe_clear_user() yet.

- Eric

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Eric Biggers]

Hi Al,

On Sun, Mar 12, 2017 at 01:24:15AM +, Al Viro wrote:
> On Sat, Mar 11, 2017 at 01:45:55PM -0800, Eric Biggers wrote:
> > From: Eric Biggers 
> > 
> > I found that statx() was significantly slower than stat().  As a
> > microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
> > file to the same with statx() passed a NULL path:
> 
> Umm...
> 

Well, it's a silly benchmark, but stat performance is important, and usually
things are cached already so most of the time is just overhead --- which this
measures.  And since nothing actually uses statx() yet, you can't do a benchmark
just by running some command like 'git status' or whatever.

> > +   tmp.stx_dev_major = MAJOR(stat->dev);
> > +   tmp.stx_dev_minor = MINOR(stat->dev);
> > +   memset(tmp.__spare2, 0, sizeof(tmp.__spare2));
> > +
> > +   return copy_to_user(buffer, , sizeof(tmp)) ? -EFAULT : 0;
> 
> That relies upon there being no padding in the damn structure.
> It's true and probably will be true on any target, but
>   a) it's bloody well worth stating explicitly
> and
>   b) struct statx tmp = {.stx_mask = stat->result_mask};
> will get rid of those memset() you've got there by implicit
> zeroing of fields missing in partial structure initializer.
> Padding is *not* included into that, but you are relying upon
> having no padding anyway...

If padding is a concern at all (AFAICS it's not actually an issue now with
struct statx, but people tend to have different opinions on how careful they
want to be with padding), then I think we'll just have to start by memsetting
the whole struct to 0.

stat() actually does that, except that it's overridden on some architectures,
like x86, to only memset the actual reserved fields.  I had kind of assumed that
as long as we're defining a new struct that has the same layout on all
architectures, with no padding, that we'd want to take the opportunity to only
memset the actual reserved fields, to make it a bit faster.  And yes, a
designated initializer would make this look a bit nicer, though it might have to
be changed later if any future statx() extensions involve fields that are
unioned or conditionally written.

Note that another approach would be to copy the fields individually, but with
unsafe_put_user() instead of __put_user().  That would avoid the overhead of
user_access_begin()/user_access_end() which was the main performance problem.
However, the infrastructure around this doesn't seem to be ready quite yet:
there aren't good implementations of unsafe_put_user() yet, nor is there an
unsafe_clear_user() yet.

- Eric

Re: [RFC] Add option to mount only a pids subset

[Al Viro]

On Sun, Mar 12, 2017 at 01:54:38AM +, Al Viro wrote:
> On Tue, Mar 07, 2017 at 12:05:16AM +0100, Alexey Gladkov wrote:
> 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 83de8b6..5bd1b84 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1759,6 +1759,8 @@ struct super_operations {
> > int (*thaw_super) (struct super_block *);
> > int (*unfreeze_fs) (struct super_block *);
> > int (*statfs) (struct dentry *, struct kstatfs *);
> > +   int (*getattr_fs) (struct vfsmount *, struct dentry *, struct kstat *);
> > +   int (*mount_fs) (struct vfsmount *, int *, char *);
> > int (*remount_fs) (struct super_block *, int *, char *);
> > void (*umount_begin) (struct super_block *);
> >  
> > diff --git a/include/linux/mount.h b/include/linux/mount.h
> > index 1172cce..4bd364e 100644
> > --- a/include/linux/mount.h
> > +++ b/include/linux/mount.h
> > @@ -67,6 +67,7 @@ struct vfsmount {
> > struct dentry *mnt_root;/* root of the mounted tree */
> > struct super_block *mnt_sb; /* pointer to superblock */
> > int mnt_flags;
> > +   void *fs_data;  /* fs-specific data */
> 
>   No.  This is really asking for trouble - you *can't* hang
> fs-specific data structures off vfsmount.  Lifetimes make no
> sense whatsoever.
>
>   BTW, your patch leaks supreblock reference on failure, and
> is kludgy as hell wrt to what it's doing in procfs itself, but
> that's secondary - the quoted part is enough for a hard NAK on
> architectural grounds.  Don't go there.

PS: AFAICS, simple mount --bind of your pid-only mount will suddenly
expose the full thing.  And as for the lifetimes making no sense...
note that you are simply not freeing these structures of yours.
Try to handle that and you'll get a serious PITA all over the
place.

What are you trying to achieve, anyway?  Why not add a second vfsmount
pointer per pid_namespace and make it initialized on demand, at the
first attempt of no-pid mount?  Just have a separate no-pid instance
created for those namespaces where it had been asked for, with
separate superblock and dentry tree not containing anything other
that pid-only parts + self + thread-self...

Re: [RFC] Add option to mount only a pids subset

[Al Viro]

On Sun, Mar 12, 2017 at 01:54:38AM +, Al Viro wrote:
> On Tue, Mar 07, 2017 at 12:05:16AM +0100, Alexey Gladkov wrote:
> 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 83de8b6..5bd1b84 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1759,6 +1759,8 @@ struct super_operations {
> > int (*thaw_super) (struct super_block *);
> > int (*unfreeze_fs) (struct super_block *);
> > int (*statfs) (struct dentry *, struct kstatfs *);
> > +   int (*getattr_fs) (struct vfsmount *, struct dentry *, struct kstat *);
> > +   int (*mount_fs) (struct vfsmount *, int *, char *);
> > int (*remount_fs) (struct super_block *, int *, char *);
> > void (*umount_begin) (struct super_block *);
> >  
> > diff --git a/include/linux/mount.h b/include/linux/mount.h
> > index 1172cce..4bd364e 100644
> > --- a/include/linux/mount.h
> > +++ b/include/linux/mount.h
> > @@ -67,6 +67,7 @@ struct vfsmount {
> > struct dentry *mnt_root;/* root of the mounted tree */
> > struct super_block *mnt_sb; /* pointer to superblock */
> > int mnt_flags;
> > +   void *fs_data;  /* fs-specific data */
> 
>   No.  This is really asking for trouble - you *can't* hang
> fs-specific data structures off vfsmount.  Lifetimes make no
> sense whatsoever.
>
>   BTW, your patch leaks supreblock reference on failure, and
> is kludgy as hell wrt to what it's doing in procfs itself, but
> that's secondary - the quoted part is enough for a hard NAK on
> architectural grounds.  Don't go there.

PS: AFAICS, simple mount --bind of your pid-only mount will suddenly
expose the full thing.  And as for the lifetimes making no sense...
note that you are simply not freeing these structures of yours.
Try to handle that and you'll get a serious PITA all over the
place.

What are you trying to achieve, anyway?  Why not add a second vfsmount
pointer per pid_namespace and make it initialized on demand, at the
first attempt of no-pid mount?  Just have a separate no-pid instance
created for those namespaces where it had been asked for, with
separate superblock and dentry tree not containing anything other
that pid-only parts + self + thread-self...

Re: Synaptics RMI4 touchpad regression in 4.11-rc1

[Cameron Gutman]

On 03/11/2017 05:55 PM, Cameron Gutman wrote:
> 
> The affected machine is an XPS 13 9443 running Fedora 25 with 4.11-rc1 
> and libinput 1.6.3-3.fc25 (latest in F25).
> 

Oops, that's 9343, not 9443.

DMI: Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016

Re: Synaptics RMI4 touchpad regression in 4.11-rc1

[Cameron Gutman]

On 03/11/2017 05:55 PM, Cameron Gutman wrote:
> 
> The affected machine is an XPS 13 9443 running Fedora 25 with 4.11-rc1 
> and libinput 1.6.3-3.fc25 (latest in F25).
> 

Oops, that's 9343, not 9443.

DMI: Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016

RE: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Wang, Wei W]

On 03/11/2017 10:10 PM, Matthew Wilcox wrote:
> On Sat, Mar 11, 2017 at 07:59:31PM +0800, Wei Wang wrote:
> > I'm thinking what if the guest needs to transfer these much physically
> > continuous memory to host: 1GB+2MB+64KB+32KB+16KB+4KB.
> > Is it going to use Six 64-bit chunks? Would it be simpler if we just
> > use the 128-bit chunk format (we can drop the previous normal 64-bit
> > format)?
> 
> Is that a likely thing for the guest to need to do though?  Freeing a 1GB 
> page is
> much more liikely, IMO.

Yes, I think it's very possible. The host can ask for any number of pages (e.g. 
1.5GB) that the guest can afford.  Also, the ballooned 1.5G memory is not 
guaranteed to be continuous in any pattern like 1GB+512MB. That's why we need 
to use a bitmap to draw the whole picture first, and then seek for continuous 
bits to chunk.

Best,
Wei

RE: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Wang, Wei W]

On 03/11/2017 10:10 PM, Matthew Wilcox wrote:
> On Sat, Mar 11, 2017 at 07:59:31PM +0800, Wei Wang wrote:
> > I'm thinking what if the guest needs to transfer these much physically
> > continuous memory to host: 1GB+2MB+64KB+32KB+16KB+4KB.
> > Is it going to use Six 64-bit chunks? Would it be simpler if we just
> > use the 128-bit chunk format (we can drop the previous normal 64-bit
> > format)?
> 
> Is that a likely thing for the guest to need to do though?  Freeing a 1GB 
> page is
> much more liikely, IMO.

Yes, I think it's very possible. The host can ask for any number of pages (e.g. 
1.5GB) that the guest can afford.  Also, the ballooned 1.5G memory is not 
guaranteed to be continuous in any pattern like 1GB+512MB. That's why we need 
to use a bitmap to draw the whole picture first, and then seek for continuous 
bits to chunk.

Best,
Wei

Synaptics RMI4 touchpad regression in 4.11-rc1

[Cameron Gutman]

Hi,

Beginning in 4.11-rc1, it looks like RMI4 is binding to my XPS 13 9443's
Synaptics touchpad and dropping some errors into dmesg. Here are the
messages that seem RMI-related:

rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader version
rmi4_f34: probe of rmi4-00.fn34 failed with error -22
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: 
TM3038-001, fw id: 1832324
input: Synaptics TM3038-001 as 
/devices/pci:00/INT3433:00/i2c-7/i2c-DLL0665:01/0018:06CB:76AD.0001/input/input19
hid-rmi 0018:06CB:76AD.0001: input,hidraw0: I2C HID v1.00 Mouse [DLL0665:01 
06CB:76AD] on i2c-DLL0665:01

Since "Unrecognized bootloader version" isn't a really helpful message, I
applied the attached patch to print the bootloader version which gave
me the following:

rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader version: 16

I don't really know what to make of that. It seem very different than the
values the existing code expects.


Compared to hid-multitouch, the RMI stack seems to have completely broken
palm rejection and introduced some random jumpiness during fine pointing
motions. I don't know if these issues are caused by the above errors or
are a separate issue.

The affected machine is an XPS 13 9443 running Fedora 25 with 4.11-rc1 
and libinput 1.6.3-3.fc25 (latest in F25).

Let me know any additional info you'd like.

Regards,
Cameron


diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 56c6c39..b458cb3 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -1369,8 +1369,8 @@ int rmi_f34v7_probe(struct f34_data *f34)
} else if (f34->bootloader_id[1] == 7) {
f34->bl_version = 7;
} else {
-   dev_err(>fn->dev, "%s: Unrecognized bootloader version\n",
-   __func__);
+   dev_err(>fn->dev, "%s: Unrecognized bootloader version: 
%u\n",
+   __func__, f34->bootloader_id[1]);
return -EINVAL;
}

Synaptics RMI4 touchpad regression in 4.11-rc1

[Cameron Gutman]

Hi,

Beginning in 4.11-rc1, it looks like RMI4 is binding to my XPS 13 9443's
Synaptics touchpad and dropping some errors into dmesg. Here are the
messages that seem RMI-related:

rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader version
rmi4_f34: probe of rmi4-00.fn34 failed with error -22
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: 
TM3038-001, fw id: 1832324
input: Synaptics TM3038-001 as 
/devices/pci:00/INT3433:00/i2c-7/i2c-DLL0665:01/0018:06CB:76AD.0001/input/input19
hid-rmi 0018:06CB:76AD.0001: input,hidraw0: I2C HID v1.00 Mouse [DLL0665:01 
06CB:76AD] on i2c-DLL0665:01

Since "Unrecognized bootloader version" isn't a really helpful message, I
applied the attached patch to print the bootloader version which gave
me the following:

rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader version: 16

I don't really know what to make of that. It seem very different than the
values the existing code expects.


Compared to hid-multitouch, the RMI stack seems to have completely broken
palm rejection and introduced some random jumpiness during fine pointing
motions. I don't know if these issues are caused by the above errors or
are a separate issue.

The affected machine is an XPS 13 9443 running Fedora 25 with 4.11-rc1 
and libinput 1.6.3-3.fc25 (latest in F25).

Let me know any additional info you'd like.

Regards,
Cameron


diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index 56c6c39..b458cb3 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -1369,8 +1369,8 @@ int rmi_f34v7_probe(struct f34_data *f34)
} else if (f34->bootloader_id[1] == 7) {
f34->bl_version = 7;
} else {
-   dev_err(>fn->dev, "%s: Unrecognized bootloader version\n",
-   __func__);
+   dev_err(>fn->dev, "%s: Unrecognized bootloader version: 
%u\n",
+   __func__, f34->bootloader_id[1]);
return -EINVAL;
}

Re: [RFC] Add option to mount only a pids subset

[Al Viro]

On Tue, Mar 07, 2017 at 12:05:16AM +0100, Alexey Gladkov wrote:

> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 83de8b6..5bd1b84 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1759,6 +1759,8 @@ struct super_operations {
>   int (*thaw_super) (struct super_block *);
>   int (*unfreeze_fs) (struct super_block *);
>   int (*statfs) (struct dentry *, struct kstatfs *);
> + int (*getattr_fs) (struct vfsmount *, struct dentry *, struct kstat *);
> + int (*mount_fs) (struct vfsmount *, int *, char *);
>   int (*remount_fs) (struct super_block *, int *, char *);
>   void (*umount_begin) (struct super_block *);
>  
> diff --git a/include/linux/mount.h b/include/linux/mount.h
> index 1172cce..4bd364e 100644
> --- a/include/linux/mount.h
> +++ b/include/linux/mount.h
> @@ -67,6 +67,7 @@ struct vfsmount {
>   struct dentry *mnt_root;/* root of the mounted tree */
>   struct super_block *mnt_sb; /* pointer to superblock */
>   int mnt_flags;
> + void *fs_data;  /* fs-specific data */

No.  This is really asking for trouble - you *can't* hang
fs-specific data structures off vfsmount.  Lifetimes make no
sense whatsoever.

BTW, your patch leaks supreblock reference on failure, and
is kludgy as hell wrt to what it's doing in procfs itself, but
that's secondary - the quoted part is enough for a hard NAK on
architectural grounds.  Don't go there.

Re: [RFC] Add option to mount only a pids subset

[Al Viro]

On Tue, Mar 07, 2017 at 12:05:16AM +0100, Alexey Gladkov wrote:

> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 83de8b6..5bd1b84 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1759,6 +1759,8 @@ struct super_operations {
>   int (*thaw_super) (struct super_block *);
>   int (*unfreeze_fs) (struct super_block *);
>   int (*statfs) (struct dentry *, struct kstatfs *);
> + int (*getattr_fs) (struct vfsmount *, struct dentry *, struct kstat *);
> + int (*mount_fs) (struct vfsmount *, int *, char *);
>   int (*remount_fs) (struct super_block *, int *, char *);
>   void (*umount_begin) (struct super_block *);
>  
> diff --git a/include/linux/mount.h b/include/linux/mount.h
> index 1172cce..4bd364e 100644
> --- a/include/linux/mount.h
> +++ b/include/linux/mount.h
> @@ -67,6 +67,7 @@ struct vfsmount {
>   struct dentry *mnt_root;/* root of the mounted tree */
>   struct super_block *mnt_sb; /* pointer to superblock */
>   int mnt_flags;
> + void *fs_data;  /* fs-specific data */

No.  This is really asking for trouble - you *can't* hang
fs-specific data structures off vfsmount.  Lifetimes make no
sense whatsoever.

BTW, your patch leaks supreblock reference on failure, and
is kludgy as hell wrt to what it's doing in procfs itself, but
that's secondary - the quoted part is enough for a hard NAK on
architectural grounds.  Don't go there.

[PATCH 2/3] Staging: ks7010: ks_hostif.*: Use preferred 'u16' kernel type over 'uint16_t'

[Shiva Kerdel]

Fix prefer kernel type 'u16' over 'uint16_t' checks.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/ks7010/ks_hostif.c |  20 +++---
 drivers/staging/ks7010/ks_hostif.h | 134 ++---
 2 files changed, 77 insertions(+), 77 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 0d6bbe61131c..2cbb04305f33 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -509,8 +509,8 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
struct net_device *dev = priv->net_dev;
uint32_t mib_status;
uint32_t mib_attribute;
-   uint16_t mib_val_size;
-   uint16_t mib_val_type;
+   u16 mib_val_size;
+   u16 mib_val_type;
 
DPRINTK(3, "\n");
 
@@ -901,7 +901,7 @@ void hostif_ps_adhoc_set_confirm(struct ks_wlan_private 
*priv)
 static
 void hostif_infrastructure_set_confirm(struct ks_wlan_private *priv)
 {
-   uint16_t result_code;
+   u16 result_code;
 
DPRINTK(3, "\n");
result_code = get_WORD(priv);
@@ -1364,7 +1364,7 @@ static
 void hostif_ps_adhoc_set_request(struct ks_wlan_private *priv)
 {
struct hostif_ps_adhoc_set_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "\n");
 
@@ -1407,7 +1407,7 @@ static
 void hostif_infrastructure_set_request(struct ks_wlan_private *priv)
 {
struct hostif_infrastructure_set_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "ssid.size=%d\n", priv->reg.ssid.size);
 
@@ -1473,7 +1473,7 @@ void hostif_infrastructure_set_request(struct 
ks_wlan_private *priv)
 static void hostif_infrastructure_set2_request(struct ks_wlan_private *priv)
 {
struct hostif_infrastructure_set2_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(2, "ssid.size=%d\n", priv->reg.ssid.size);
 
@@ -1542,7 +1542,7 @@ static
 void hostif_adhoc_set_request(struct ks_wlan_private *priv)
 {
struct hostif_adhoc_set_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "\n");
 
@@ -1587,7 +1587,7 @@ static
 void hostif_adhoc_set2_request(struct ks_wlan_private *priv)
 {
struct hostif_adhoc_set2_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "\n");
 
@@ -1919,7 +1919,7 @@ struct wpa_suite_t {
 
 struct rsn_mode_t {
uint32_t rsn_mode;
-   uint16_t rsn_capability;
+   u16 rsn_capability;
 } __packed;
 
 static
@@ -2382,7 +2382,7 @@ static
 void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
 {
struct pmk_cache_t {
-   uint16_t size;
+   u16 size;
struct {
u8 bssid[ETH_ALEN];
u8 pmkid[IW_PMKID_LEN];
diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index be8ded44c1ac..5b2120293967 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -62,27 +62,27 @@
  */
 
 struct hostif_hdr {
-   uint16_t size;
-   uint16_t event;
+   u16 size;
+   u16 event;
 } __packed;
 
 struct hostif_data_request_t {
struct hostif_hdr header;
-   uint16_t auth_type;
+   u16 auth_type;
 #define TYPE_DATA 0x
 #define TYPE_AUTH 0x0001
-   uint16_t reserved;
+   u16 reserved;
u8 data[0];
 } __packed;
 
 struct hostif_data_indication_t {
struct hostif_hdr header;
-   uint16_t auth_type;
+   u16 auth_type;
 /* #define TYPE_DATA 0x */
 #define TYPE_PMK1 0x0001
 #define TYPE_GMK1 0x0002
 #define TYPE_GMK2 0x0003
-   uint16_t reserved;
+   u16 reserved;
u8 data[0];
 } __packed;
 
@@ -147,8 +147,8 @@ struct hostif_mib_get_request_t {
 } __packed;
 
 struct hostif_mib_value_t {
-   uint16_t size;
-   uint16_t type;
+   u16 size;
+   u16 type;
 #define MIB_VALUE_TYPE_NULL 0
 #define MIB_VALUE_TYPE_INT  1
 #define MIB_VALUE_TYPE_BOOL 2
@@ -208,12 +208,12 @@ enum {
 
 struct hostif_power_mngmt_confirm_t {
struct hostif_hdr header;
-   uint16_t result_code;
+   u16 result_code;
 } __packed;
 
 struct hostif_start_request_t {
struct hostif_hdr header;
-   uint16_t mode;
+   u16 mode;
 #define MODE_PSEUDO_ADHOC   0
 #define MODE_INFRASTRUCTURE 1
 #define MODE_AP 2  /* not used */
@@ -222,7 +222,7 @@ struct hostif_start_request_t {
 
 struct hostif_start_confirm_t {
struct hostif_hdr header;
-   uint16_t result_code;
+   u16 result_code;
 } __packed;
 
 #define SSID_MAX_SIZE 32
@@ -240,7 +240,7 @@ struct rate_set8_t {
 } __packed;
 
 struct FhParms_t {
-   uint16_t dwellTime;
+   u16 dwellTime;
u8 hopSet;
u8 hopPattern;
u8 hopIndex;
@@ -253,12 +253,12 @@ struct DsParms_t {
 struct CfParms_t {
u8 count;
u8 period;
-   uint16_t 

[PATCH 2/3] Staging: ks7010: ks_hostif.*: Use preferred 'u16' kernel type over 'uint16_t'

[Shiva Kerdel]

Fix prefer kernel type 'u16' over 'uint16_t' checks.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/ks7010/ks_hostif.c |  20 +++---
 drivers/staging/ks7010/ks_hostif.h | 134 ++---
 2 files changed, 77 insertions(+), 77 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 0d6bbe61131c..2cbb04305f33 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -509,8 +509,8 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
struct net_device *dev = priv->net_dev;
uint32_t mib_status;
uint32_t mib_attribute;
-   uint16_t mib_val_size;
-   uint16_t mib_val_type;
+   u16 mib_val_size;
+   u16 mib_val_type;
 
DPRINTK(3, "\n");
 
@@ -901,7 +901,7 @@ void hostif_ps_adhoc_set_confirm(struct ks_wlan_private 
*priv)
 static
 void hostif_infrastructure_set_confirm(struct ks_wlan_private *priv)
 {
-   uint16_t result_code;
+   u16 result_code;
 
DPRINTK(3, "\n");
result_code = get_WORD(priv);
@@ -1364,7 +1364,7 @@ static
 void hostif_ps_adhoc_set_request(struct ks_wlan_private *priv)
 {
struct hostif_ps_adhoc_set_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "\n");
 
@@ -1407,7 +1407,7 @@ static
 void hostif_infrastructure_set_request(struct ks_wlan_private *priv)
 {
struct hostif_infrastructure_set_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "ssid.size=%d\n", priv->reg.ssid.size);
 
@@ -1473,7 +1473,7 @@ void hostif_infrastructure_set_request(struct 
ks_wlan_private *priv)
 static void hostif_infrastructure_set2_request(struct ks_wlan_private *priv)
 {
struct hostif_infrastructure_set2_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(2, "ssid.size=%d\n", priv->reg.ssid.size);
 
@@ -1542,7 +1542,7 @@ static
 void hostif_adhoc_set_request(struct ks_wlan_private *priv)
 {
struct hostif_adhoc_set_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "\n");
 
@@ -1587,7 +1587,7 @@ static
 void hostif_adhoc_set2_request(struct ks_wlan_private *priv)
 {
struct hostif_adhoc_set2_request_t *pp;
-   uint16_t capability;
+   u16 capability;
 
DPRINTK(3, "\n");
 
@@ -1919,7 +1919,7 @@ struct wpa_suite_t {
 
 struct rsn_mode_t {
uint32_t rsn_mode;
-   uint16_t rsn_capability;
+   u16 rsn_capability;
 } __packed;
 
 static
@@ -2382,7 +2382,7 @@ static
 void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
 {
struct pmk_cache_t {
-   uint16_t size;
+   u16 size;
struct {
u8 bssid[ETH_ALEN];
u8 pmkid[IW_PMKID_LEN];
diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index be8ded44c1ac..5b2120293967 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -62,27 +62,27 @@
  */
 
 struct hostif_hdr {
-   uint16_t size;
-   uint16_t event;
+   u16 size;
+   u16 event;
 } __packed;
 
 struct hostif_data_request_t {
struct hostif_hdr header;
-   uint16_t auth_type;
+   u16 auth_type;
 #define TYPE_DATA 0x
 #define TYPE_AUTH 0x0001
-   uint16_t reserved;
+   u16 reserved;
u8 data[0];
 } __packed;
 
 struct hostif_data_indication_t {
struct hostif_hdr header;
-   uint16_t auth_type;
+   u16 auth_type;
 /* #define TYPE_DATA 0x */
 #define TYPE_PMK1 0x0001
 #define TYPE_GMK1 0x0002
 #define TYPE_GMK2 0x0003
-   uint16_t reserved;
+   u16 reserved;
u8 data[0];
 } __packed;
 
@@ -147,8 +147,8 @@ struct hostif_mib_get_request_t {
 } __packed;
 
 struct hostif_mib_value_t {
-   uint16_t size;
-   uint16_t type;
+   u16 size;
+   u16 type;
 #define MIB_VALUE_TYPE_NULL 0
 #define MIB_VALUE_TYPE_INT  1
 #define MIB_VALUE_TYPE_BOOL 2
@@ -208,12 +208,12 @@ enum {
 
 struct hostif_power_mngmt_confirm_t {
struct hostif_hdr header;
-   uint16_t result_code;
+   u16 result_code;
 } __packed;
 
 struct hostif_start_request_t {
struct hostif_hdr header;
-   uint16_t mode;
+   u16 mode;
 #define MODE_PSEUDO_ADHOC   0
 #define MODE_INFRASTRUCTURE 1
 #define MODE_AP 2  /* not used */
@@ -222,7 +222,7 @@ struct hostif_start_request_t {
 
 struct hostif_start_confirm_t {
struct hostif_hdr header;
-   uint16_t result_code;
+   u16 result_code;
 } __packed;
 
 #define SSID_MAX_SIZE 32
@@ -240,7 +240,7 @@ struct rate_set8_t {
 } __packed;
 
 struct FhParms_t {
-   uint16_t dwellTime;
+   u16 dwellTime;
u8 hopSet;
u8 hopPattern;
u8 hopIndex;
@@ -253,12 +253,12 @@ struct DsParms_t {
 struct CfParms_t {
u8 count;
u8 period;
-   uint16_t maxDuration;
- 

[PATCH 3/3] Staging: ks7010: ks_hostif.*: Use preferred 'u32' kernel type over 'uint32_t'

[Shiva Kerdel]

Fix prefer kernel type 'u32' over 'uint32_t' checks.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/ks7010/ks_hostif.c | 18 +-
 drivers/staging/ks7010/ks_hostif.h | 30 +++---
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 2cbb04305f33..b1fff903bce6 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -507,8 +507,8 @@ static
 void hostif_mib_get_confirm(struct ks_wlan_private *priv)
 {
struct net_device *dev = priv->net_dev;
-   uint32_t mib_status;
-   uint32_t mib_attribute;
+   u32 mib_status;
+   u32 mib_attribute;
u16 mib_val_size;
u16 mib_val_type;
 
@@ -587,8 +587,8 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
 static
 void hostif_mib_set_confirm(struct ks_wlan_private *priv)
 {
-   uint32_t mib_status;/* +04 MIB Status */
-   uint32_t mib_attribute; /* +08 MIB attribute */
+   u32 mib_status; /* +04 MIB Status */
+   u32 mib_attribute;  /* +08 MIB attribute */
 
DPRINTK(3, "\n");
 
@@ -1864,7 +1864,7 @@ void hostif_receive(struct ks_wlan_private *priv, 
unsigned char *p,
 static
 void hostif_sme_set_wep(struct ks_wlan_private *priv, int type)
 {
-   uint32_t val;
+   u32 val;
 
switch (type) {
case SME_WEP_INDEX_REQUEST:
@@ -1918,7 +1918,7 @@ struct wpa_suite_t {
 } __packed;
 
 struct rsn_mode_t {
-   uint32_t rsn_mode;
+   u32 rsn_mode;
u16 rsn_capability;
 } __packed;
 
@@ -1927,7 +1927,7 @@ void hostif_sme_set_rsn(struct ks_wlan_private *priv, int 
type)
 {
struct wpa_suite_t wpa_suite;
struct rsn_mode_t rsn_mode;
-   uint32_t val;
+   u32 val;
 
memset(_suite, 0, sizeof(wpa_suite));
 
@@ -2323,7 +2323,7 @@ void hostif_sme_sleep_set(struct ks_wlan_private *priv)
 static
 void hostif_sme_set_key(struct ks_wlan_private *priv, int type)
 {
-   uint32_t val;
+   u32 val;
 
switch (type) {
case SME_SET_FLAG:
@@ -2413,7 +2413,7 @@ void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
 static
 void hostif_sme_execute(struct ks_wlan_private *priv, int event)
 {
-   uint32_t val;
+   u32 val;
 
DPRINTK(3, "event=%d\n", event);
switch (event) {
diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index 5b2120293967..02b483afff5a 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -143,7 +143,7 @@ struct channel_list_t {
 
 struct hostif_mib_get_request_t {
struct hostif_hdr header;
-   uint32_t mib_attribute;
+   u32 mib_attribute;
 } __packed;
 
 struct hostif_mib_value_t {
@@ -159,36 +159,36 @@ struct hostif_mib_value_t {
 
 struct hostif_mib_get_confirm_t {
struct hostif_hdr header;
-   uint32_t mib_status;
+   u32 mib_status;
 #define MIB_SUCCESS0
 #define MIB_INVALID1
 #define MIB_READ_ONLY  2
 #define MIB_WRITE_ONLY 3
-   uint32_t mib_attribute;
+   u32 mib_attribute;
struct hostif_mib_value_t mib_value;
 } __packed;
 
 struct hostif_mib_set_request_t {
struct hostif_hdr header;
-   uint32_t mib_attribute;
+   u32 mib_attribute;
struct hostif_mib_value_t mib_value;
 } __packed;
 
 struct hostif_mib_set_confirm_t {
struct hostif_hdr header;
-   uint32_t mib_status;
-   uint32_t mib_attribute;
+   u32 mib_status;
+   u32 mib_attribute;
 } __packed;
 
 struct hostif_power_mngmt_request_t {
struct hostif_hdr header;
-   uint32_t mode;
+   u32 mode;
 #define POWER_ACTIVE  1
 #define POWER_SAVE2
-   uint32_t wake_up;
+   u32 wake_up;
 #define SLEEP_FALSE 0
 #define SLEEP_TRUE  1  /* not used */
-   uint32_t receiveDTIMs;
+   u32 receiveDTIMs;
 #define DTIM_FALSE 0
 #define DTIM_TRUE  1
 } __packed;
@@ -480,8 +480,8 @@ struct hostif_bss_scan_request_t {
 #define ACTIVE_SCAN  0
 #define PASSIVE_SCAN 1
u8 pad[3];
-   uint32_t ch_time_min;
-   uint32_t ch_time_max;
+   u32 ch_time_min;
+   u32 ch_time_max;
struct channel_list_t channel_list;
struct ssid_t ssid;
 } __packed;
@@ -506,10 +506,10 @@ struct hostif_phy_information_confirm_t {
u8 sq;
u8 noise;
u8 link_speed;
-   uint32_t tx_frame;
-   uint32_t rx_frame;
-   uint32_t tx_error;
-   uint32_t rx_error;
+   u32 tx_frame;
+   u32 rx_frame;
+   u32 tx_error;
+   u32 rx_error;
 } __packed;
 
 /* sleep mode */
-- 
2.12.0

[PATCH 1/3] Staging: ks7010: ks_*: Use preferred 'u8' kernel type over 'uint8_t'

[Shiva Kerdel]

Fix prefer kernel type 'u8' over 'uint8_t' checks.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/ks7010/ks_hostif.c   |   4 +-
 drivers/staging/ks7010/ks_hostif.h   | 114 +--
 drivers/staging/ks7010/ks_wlan_net.c |   2 +-
 3 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 6522bc3d30d5..0d6bbe61131c 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2384,8 +2384,8 @@ void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
struct pmk_cache_t {
uint16_t size;
struct {
-   uint8_t bssid[ETH_ALEN];
-   uint8_t pmkid[IW_PMKID_LEN];
+   u8 bssid[ETH_ALEN];
+   u8 pmkid[IW_PMKID_LEN];
} __packed list[PMK_LIST_MAX];
} __packed pmkcache;
struct pmk_t *pmk;
diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index 35bfadf4665d..be8ded44c1ac 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -72,7 +72,7 @@ struct hostif_data_request_t {
 #define TYPE_DATA 0x
 #define TYPE_AUTH 0x0001
uint16_t reserved;
-   uint8_t data[0];
+   u8 data[0];
 } __packed;
 
 struct hostif_data_indication_t {
@@ -83,14 +83,14 @@ struct hostif_data_indication_t {
 #define TYPE_GMK1 0x0002
 #define TYPE_GMK2 0x0003
uint16_t reserved;
-   uint8_t data[0];
+   u8 data[0];
 } __packed;
 
 #define CHANNEL_LIST_MAX_SIZE 14
 struct channel_list_t {
-   uint8_t size;
-   uint8_t body[CHANNEL_LIST_MAX_SIZE];
-   uint8_t pad;
+   u8 size;
+   u8 body[CHANNEL_LIST_MAX_SIZE];
+   u8 pad;
 } __packed;
 
 /* MIB Attribute */
@@ -154,7 +154,7 @@ struct hostif_mib_value_t {
 #define MIB_VALUE_TYPE_BOOL 2
 #define MIB_VALUE_TYPE_COUNT32  3
 #define MIB_VALUE_TYPE_OSTRING  4
-   uint8_t body[0];
+   u8 body[0];
 } __packed;
 
 struct hostif_mib_get_confirm_t {
@@ -227,32 +227,32 @@ struct hostif_start_confirm_t {
 
 #define SSID_MAX_SIZE 32
 struct ssid_t {
-   uint8_t size;
-   uint8_t body[SSID_MAX_SIZE];
-   uint8_t ssid_pad;
+   u8 size;
+   u8 body[SSID_MAX_SIZE];
+   u8 ssid_pad;
 } __packed;
 
 #define RATE_SET_MAX_SIZE 16
 struct rate_set8_t {
-   uint8_t size;
-   uint8_t body[8];
-   uint8_t rate_pad;
+   u8 size;
+   u8 body[8];
+   u8 rate_pad;
 } __packed;
 
 struct FhParms_t {
uint16_t dwellTime;
-   uint8_t hopSet;
-   uint8_t hopPattern;
-   uint8_t hopIndex;
+   u8 hopSet;
+   u8 hopPattern;
+   u8 hopIndex;
 } __packed;
 
 struct DsParms_t {
-   uint8_t channel;
+   u8 channel;
 } __packed;
 
 struct CfParms_t {
-   uint8_t count;
-   uint8_t period;
+   u8 count;
+   u8 period;
uint16_t maxDuration;
uint16_t durRemaining;
 } __packed;
@@ -262,27 +262,27 @@ struct IbssParms_t {
 } __packed;
 
 struct rsn_t {
-   uint8_t size;
+   u8 size;
 #define RSN_BODY_SIZE 64
-   uint8_t body[RSN_BODY_SIZE];
+   u8 body[RSN_BODY_SIZE];
 } __packed;
 
 struct ErpParams_t {
-   uint8_t erp_info;
+   u8 erp_info;
 } __packed;
 
 struct rate_set16_t {
-   uint8_t size;
-   uint8_t body[16];
-   uint8_t rate_pad;
+   u8 size;
+   u8 body[16];
+   u8 rate_pad;
 } __packed;
 
 struct ap_info_t {
-   uint8_t bssid[6];   /* +00 */
-   uint8_t rssi;   /* +06 */
-   uint8_t sq; /* +07 */
-   uint8_t noise;  /* +08 */
-   uint8_t pad0;   /* +09 */
+   u8 bssid[6];/* +00 */
+   u8 rssi;/* +06 */
+   u8 sq;  /* +07 */
+   u8 noise;   /* +08 */
+   u8 pad0;/* +09 */
uint16_t beacon_period; /* +10 */
uint16_t capability;/* +12 */
 #define BSS_CAP_ESS BIT(0)
@@ -295,21 +295,21 @@ struct ap_info_t {
 #define BSS_CAP_CHANNEL_AGILITY BIT(7)
 #define BSS_CAP_SHORT_SLOT_TIME BIT(10)
 #define BSS_CAP_DSSS_OFDM   BIT(13)
-   uint8_t frame_type; /* +14 */
-   uint8_t ch_info;/* +15 */
+   u8 frame_type;  /* +14 */
+   u8 ch_info; /* +15 */
 #define FRAME_TYPE_BEACON  0x80
 #define FRAME_TYPE_PROBE_RESP  0x50
uint16_t body_size; /* +16 */
-   uint8_t body[1024]; /* +18 */
+   u8 body[1024];  /* +18 */
/* +1032 */
 } __packed;
 
 struct link_ap_info_t {
-   uint8_t bssid[6];   /* +00 */
-   uint8_t rssi;   /* +06 */
-   uint8_t sq; /* +07 */
-   uint8_t noise;  /* +08 */
-   uint8_t pad0;   /* +09 */
+   u8 bssid[6];/* +00 */
+   u8 rssi;/* +06 */
+   u8 sq;  /* +07 */
+   u8 noise;   /* +08 */
+   u8 pad0;/* +09 */
uint16_t beacon_period; /* +10 */
uint16_t capability;/* 

[PATCH 3/3] Staging: ks7010: ks_hostif.*: Use preferred 'u32' kernel type over 'uint32_t'

[Shiva Kerdel]

Fix prefer kernel type 'u32' over 'uint32_t' checks.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/ks7010/ks_hostif.c | 18 +-
 drivers/staging/ks7010/ks_hostif.h | 30 +++---
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 2cbb04305f33..b1fff903bce6 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -507,8 +507,8 @@ static
 void hostif_mib_get_confirm(struct ks_wlan_private *priv)
 {
struct net_device *dev = priv->net_dev;
-   uint32_t mib_status;
-   uint32_t mib_attribute;
+   u32 mib_status;
+   u32 mib_attribute;
u16 mib_val_size;
u16 mib_val_type;
 
@@ -587,8 +587,8 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
 static
 void hostif_mib_set_confirm(struct ks_wlan_private *priv)
 {
-   uint32_t mib_status;/* +04 MIB Status */
-   uint32_t mib_attribute; /* +08 MIB attribute */
+   u32 mib_status; /* +04 MIB Status */
+   u32 mib_attribute;  /* +08 MIB attribute */
 
DPRINTK(3, "\n");
 
@@ -1864,7 +1864,7 @@ void hostif_receive(struct ks_wlan_private *priv, 
unsigned char *p,
 static
 void hostif_sme_set_wep(struct ks_wlan_private *priv, int type)
 {
-   uint32_t val;
+   u32 val;
 
switch (type) {
case SME_WEP_INDEX_REQUEST:
@@ -1918,7 +1918,7 @@ struct wpa_suite_t {
 } __packed;
 
 struct rsn_mode_t {
-   uint32_t rsn_mode;
+   u32 rsn_mode;
u16 rsn_capability;
 } __packed;
 
@@ -1927,7 +1927,7 @@ void hostif_sme_set_rsn(struct ks_wlan_private *priv, int 
type)
 {
struct wpa_suite_t wpa_suite;
struct rsn_mode_t rsn_mode;
-   uint32_t val;
+   u32 val;
 
memset(_suite, 0, sizeof(wpa_suite));
 
@@ -2323,7 +2323,7 @@ void hostif_sme_sleep_set(struct ks_wlan_private *priv)
 static
 void hostif_sme_set_key(struct ks_wlan_private *priv, int type)
 {
-   uint32_t val;
+   u32 val;
 
switch (type) {
case SME_SET_FLAG:
@@ -2413,7 +2413,7 @@ void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
 static
 void hostif_sme_execute(struct ks_wlan_private *priv, int event)
 {
-   uint32_t val;
+   u32 val;
 
DPRINTK(3, "event=%d\n", event);
switch (event) {
diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index 5b2120293967..02b483afff5a 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -143,7 +143,7 @@ struct channel_list_t {
 
 struct hostif_mib_get_request_t {
struct hostif_hdr header;
-   uint32_t mib_attribute;
+   u32 mib_attribute;
 } __packed;
 
 struct hostif_mib_value_t {
@@ -159,36 +159,36 @@ struct hostif_mib_value_t {
 
 struct hostif_mib_get_confirm_t {
struct hostif_hdr header;
-   uint32_t mib_status;
+   u32 mib_status;
 #define MIB_SUCCESS0
 #define MIB_INVALID1
 #define MIB_READ_ONLY  2
 #define MIB_WRITE_ONLY 3
-   uint32_t mib_attribute;
+   u32 mib_attribute;
struct hostif_mib_value_t mib_value;
 } __packed;
 
 struct hostif_mib_set_request_t {
struct hostif_hdr header;
-   uint32_t mib_attribute;
+   u32 mib_attribute;
struct hostif_mib_value_t mib_value;
 } __packed;
 
 struct hostif_mib_set_confirm_t {
struct hostif_hdr header;
-   uint32_t mib_status;
-   uint32_t mib_attribute;
+   u32 mib_status;
+   u32 mib_attribute;
 } __packed;
 
 struct hostif_power_mngmt_request_t {
struct hostif_hdr header;
-   uint32_t mode;
+   u32 mode;
 #define POWER_ACTIVE  1
 #define POWER_SAVE2
-   uint32_t wake_up;
+   u32 wake_up;
 #define SLEEP_FALSE 0
 #define SLEEP_TRUE  1  /* not used */
-   uint32_t receiveDTIMs;
+   u32 receiveDTIMs;
 #define DTIM_FALSE 0
 #define DTIM_TRUE  1
 } __packed;
@@ -480,8 +480,8 @@ struct hostif_bss_scan_request_t {
 #define ACTIVE_SCAN  0
 #define PASSIVE_SCAN 1
u8 pad[3];
-   uint32_t ch_time_min;
-   uint32_t ch_time_max;
+   u32 ch_time_min;
+   u32 ch_time_max;
struct channel_list_t channel_list;
struct ssid_t ssid;
 } __packed;
@@ -506,10 +506,10 @@ struct hostif_phy_information_confirm_t {
u8 sq;
u8 noise;
u8 link_speed;
-   uint32_t tx_frame;
-   uint32_t rx_frame;
-   uint32_t tx_error;
-   uint32_t rx_error;
+   u32 tx_frame;
+   u32 rx_frame;
+   u32 tx_error;
+   u32 rx_error;
 } __packed;
 
 /* sleep mode */
-- 
2.12.0

[PATCH 1/3] Staging: ks7010: ks_*: Use preferred 'u8' kernel type over 'uint8_t'

[Shiva Kerdel]

Fix prefer kernel type 'u8' over 'uint8_t' checks.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/ks7010/ks_hostif.c   |   4 +-
 drivers/staging/ks7010/ks_hostif.h   | 114 +--
 drivers/staging/ks7010/ks_wlan_net.c |   2 +-
 3 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 6522bc3d30d5..0d6bbe61131c 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2384,8 +2384,8 @@ void hostif_sme_set_pmksa(struct ks_wlan_private *priv)
struct pmk_cache_t {
uint16_t size;
struct {
-   uint8_t bssid[ETH_ALEN];
-   uint8_t pmkid[IW_PMKID_LEN];
+   u8 bssid[ETH_ALEN];
+   u8 pmkid[IW_PMKID_LEN];
} __packed list[PMK_LIST_MAX];
} __packed pmkcache;
struct pmk_t *pmk;
diff --git a/drivers/staging/ks7010/ks_hostif.h 
b/drivers/staging/ks7010/ks_hostif.h
index 35bfadf4665d..be8ded44c1ac 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -72,7 +72,7 @@ struct hostif_data_request_t {
 #define TYPE_DATA 0x
 #define TYPE_AUTH 0x0001
uint16_t reserved;
-   uint8_t data[0];
+   u8 data[0];
 } __packed;
 
 struct hostif_data_indication_t {
@@ -83,14 +83,14 @@ struct hostif_data_indication_t {
 #define TYPE_GMK1 0x0002
 #define TYPE_GMK2 0x0003
uint16_t reserved;
-   uint8_t data[0];
+   u8 data[0];
 } __packed;
 
 #define CHANNEL_LIST_MAX_SIZE 14
 struct channel_list_t {
-   uint8_t size;
-   uint8_t body[CHANNEL_LIST_MAX_SIZE];
-   uint8_t pad;
+   u8 size;
+   u8 body[CHANNEL_LIST_MAX_SIZE];
+   u8 pad;
 } __packed;
 
 /* MIB Attribute */
@@ -154,7 +154,7 @@ struct hostif_mib_value_t {
 #define MIB_VALUE_TYPE_BOOL 2
 #define MIB_VALUE_TYPE_COUNT32  3
 #define MIB_VALUE_TYPE_OSTRING  4
-   uint8_t body[0];
+   u8 body[0];
 } __packed;
 
 struct hostif_mib_get_confirm_t {
@@ -227,32 +227,32 @@ struct hostif_start_confirm_t {
 
 #define SSID_MAX_SIZE 32
 struct ssid_t {
-   uint8_t size;
-   uint8_t body[SSID_MAX_SIZE];
-   uint8_t ssid_pad;
+   u8 size;
+   u8 body[SSID_MAX_SIZE];
+   u8 ssid_pad;
 } __packed;
 
 #define RATE_SET_MAX_SIZE 16
 struct rate_set8_t {
-   uint8_t size;
-   uint8_t body[8];
-   uint8_t rate_pad;
+   u8 size;
+   u8 body[8];
+   u8 rate_pad;
 } __packed;
 
 struct FhParms_t {
uint16_t dwellTime;
-   uint8_t hopSet;
-   uint8_t hopPattern;
-   uint8_t hopIndex;
+   u8 hopSet;
+   u8 hopPattern;
+   u8 hopIndex;
 } __packed;
 
 struct DsParms_t {
-   uint8_t channel;
+   u8 channel;
 } __packed;
 
 struct CfParms_t {
-   uint8_t count;
-   uint8_t period;
+   u8 count;
+   u8 period;
uint16_t maxDuration;
uint16_t durRemaining;
 } __packed;
@@ -262,27 +262,27 @@ struct IbssParms_t {
 } __packed;
 
 struct rsn_t {
-   uint8_t size;
+   u8 size;
 #define RSN_BODY_SIZE 64
-   uint8_t body[RSN_BODY_SIZE];
+   u8 body[RSN_BODY_SIZE];
 } __packed;
 
 struct ErpParams_t {
-   uint8_t erp_info;
+   u8 erp_info;
 } __packed;
 
 struct rate_set16_t {
-   uint8_t size;
-   uint8_t body[16];
-   uint8_t rate_pad;
+   u8 size;
+   u8 body[16];
+   u8 rate_pad;
 } __packed;
 
 struct ap_info_t {
-   uint8_t bssid[6];   /* +00 */
-   uint8_t rssi;   /* +06 */
-   uint8_t sq; /* +07 */
-   uint8_t noise;  /* +08 */
-   uint8_t pad0;   /* +09 */
+   u8 bssid[6];/* +00 */
+   u8 rssi;/* +06 */
+   u8 sq;  /* +07 */
+   u8 noise;   /* +08 */
+   u8 pad0;/* +09 */
uint16_t beacon_period; /* +10 */
uint16_t capability;/* +12 */
 #define BSS_CAP_ESS BIT(0)
@@ -295,21 +295,21 @@ struct ap_info_t {
 #define BSS_CAP_CHANNEL_AGILITY BIT(7)
 #define BSS_CAP_SHORT_SLOT_TIME BIT(10)
 #define BSS_CAP_DSSS_OFDM   BIT(13)
-   uint8_t frame_type; /* +14 */
-   uint8_t ch_info;/* +15 */
+   u8 frame_type;  /* +14 */
+   u8 ch_info; /* +15 */
 #define FRAME_TYPE_BEACON  0x80
 #define FRAME_TYPE_PROBE_RESP  0x50
uint16_t body_size; /* +16 */
-   uint8_t body[1024]; /* +18 */
+   u8 body[1024];  /* +18 */
/* +1032 */
 } __packed;
 
 struct link_ap_info_t {
-   uint8_t bssid[6];   /* +00 */
-   uint8_t rssi;   /* +06 */
-   uint8_t sq; /* +07 */
-   uint8_t noise;  /* +08 */
-   uint8_t pad0;   /* +09 */
+   u8 bssid[6];/* +00 */
+   u8 rssi;/* +06 */
+   u8 sq;  /* +07 */
+   u8 noise;   /* +08 */
+   u8 pad0;/* +09 */
uint16_t beacon_period; /* +10 */
uint16_t capability;/* +12 */

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Al Viro]

On Sat, Mar 11, 2017 at 01:45:55PM -0800, Eric Biggers wrote:
> From: Eric Biggers 
> 
> I found that statx() was significantly slower than stat().  As a
> microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
> file to the same with statx() passed a NULL path:

Umm...

> + struct statx tmp;
> +
> + tmp.stx_mask = stat->result_mask;
> + tmp.stx_blksize = stat->blksize;
> + tmp.stx_attributes = stat->attributes;
> + tmp.stx_nlink = stat->nlink;
> + tmp.stx_uid = from_kuid_munged(current_user_ns(), stat->uid);
> + tmp.stx_gid = from_kgid_munged(current_user_ns(), stat->gid);
> + tmp.stx_mode = stat->mode;
> + memset(tmp.__spare0, 0, sizeof(tmp.__spare0));
> + tmp.stx_ino = stat->ino;
> + tmp.stx_size = stat->size;
> + tmp.stx_blocks = stat->blocks;
> + memset(tmp.__spare1, 0, sizeof(tmp.__spare1));
> + init_statx_timestamp(_atime, >atime);
> + init_statx_timestamp(_btime, >btime);
> + init_statx_timestamp(_ctime, >ctime);
> + init_statx_timestamp(_mtime, >mtime);
> + tmp.stx_rdev_major = MAJOR(stat->rdev);
> + tmp.stx_rdev_minor = MINOR(stat->rdev);
> + tmp.stx_dev_major = MAJOR(stat->dev);
> + tmp.stx_dev_minor = MINOR(stat->dev);
> + memset(tmp.__spare2, 0, sizeof(tmp.__spare2));
> +
> + return copy_to_user(buffer, , sizeof(tmp)) ? -EFAULT : 0;

That relies upon there being no padding in the damn structure.
It's true and probably will be true on any target, but
a) it's bloody well worth stating explicitly
and
b) struct statx tmp = {.stx_mask = stat->result_mask};
will get rid of those memset() you've got there by implicit
zeroing of fields missing in partial structure initializer.
Padding is *not* included into that, but you are relying upon
having no padding anyway...

Re: [PATCH v2] statx: optimize copy of struct statx to userspace

[Al Viro]

On Sat, Mar 11, 2017 at 01:45:55PM -0800, Eric Biggers wrote:
> From: Eric Biggers 
> 
> I found that statx() was significantly slower than stat().  As a
> microbenchmark, I compared 10,000,000 invocations of fstat() on a tmpfs
> file to the same with statx() passed a NULL path:

Umm...

> + struct statx tmp;
> +
> + tmp.stx_mask = stat->result_mask;
> + tmp.stx_blksize = stat->blksize;
> + tmp.stx_attributes = stat->attributes;
> + tmp.stx_nlink = stat->nlink;
> + tmp.stx_uid = from_kuid_munged(current_user_ns(), stat->uid);
> + tmp.stx_gid = from_kgid_munged(current_user_ns(), stat->gid);
> + tmp.stx_mode = stat->mode;
> + memset(tmp.__spare0, 0, sizeof(tmp.__spare0));
> + tmp.stx_ino = stat->ino;
> + tmp.stx_size = stat->size;
> + tmp.stx_blocks = stat->blocks;
> + memset(tmp.__spare1, 0, sizeof(tmp.__spare1));
> + init_statx_timestamp(_atime, >atime);
> + init_statx_timestamp(_btime, >btime);
> + init_statx_timestamp(_ctime, >ctime);
> + init_statx_timestamp(_mtime, >mtime);
> + tmp.stx_rdev_major = MAJOR(stat->rdev);
> + tmp.stx_rdev_minor = MINOR(stat->rdev);
> + tmp.stx_dev_major = MAJOR(stat->dev);
> + tmp.stx_dev_minor = MINOR(stat->dev);
> + memset(tmp.__spare2, 0, sizeof(tmp.__spare2));
> +
> + return copy_to_user(buffer, , sizeof(tmp)) ? -EFAULT : 0;

That relies upon there being no padding in the damn structure.
It's true and probably will be true on any target, but
a) it's bloody well worth stating explicitly
and
b) struct statx tmp = {.stx_mask = stat->result_mask};
will get rid of those memset() you've got there by implicit
zeroing of fields missing in partial structure initializer.
Padding is *not* included into that, but you are relying upon
having no padding anyway...

[PATCH] ASoC: fix semicolon.cocci warnings

[kbuild test robot]

sound/soc/codecs/cs35l35.c:706:2-3: Unneeded semicolon
sound/soc/codecs/cs35l35.c:543:4-5: Unneeded semicolon
sound/soc/codecs/cs35l35.c:553:4-5: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Brian Austin 
Signed-off-by: Fengguang Wu 
---

 cs35l35.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/sound/soc/codecs/cs35l35.c
+++ b/sound/soc/codecs/cs35l35.c
@@ -540,7 +540,7 @@ static int cs35l35_hw_params(struct snd_
default:
dev_err(codec->dev, "ratio not supported\n");
return -EINVAL;
-   };
+   }
} else {
/* Only certain ratios supported in I2S MASTER Mode */
switch (sp_sclks) {
@@ -550,7 +550,7 @@ static int cs35l35_hw_params(struct snd_
default:
dev_err(codec->dev, "ratio not supported\n");
return -EINVAL;
-   };
+   }
}
ret = regmap_update_bits(cs35l35->regmap,
CS35L35_CLK_CTL3,
@@ -703,7 +703,7 @@ static int cs35l35_codec_set_sysclk(stru
default:
dev_err(codec->dev, "Invalid CLK Source\n");
return -EINVAL;
-   };
+   }
 
switch (freq) {
case 5644800:

Re: [Outreachy kernel] [PATCH] staging: android: Replace strcpy with strlcpy

[Al Viro]

On Sat, Mar 11, 2017 at 09:47:30PM +0100, Julia Lawall wrote:
> 
> 
> On Sun, 12 Mar 2017, simran singhal wrote:
> 
> > Replace strcpy with strlcpy as strcpy does not check for buffer
> > overflow.
> > This is found using Flawfinder.
> >
> > Signed-off-by: simran singhal 
> > ---
> >  drivers/staging/android/ashmem.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/android/ashmem.c 
> > b/drivers/staging/android/ashmem.c
> > index 7cbad0d..eb2f4ef 100644
> > --- a/drivers/staging/android/ashmem.c
> > +++ b/drivers/staging/android/ashmem.c
> > @@ -548,7 +548,8 @@ static int set_name(struct ashmem_area *asma, void 
> > __user *name)
> > if (unlikely(asma->file))
> > ret = -EINVAL;
> > else
> > -   strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name);
> > +   strlcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name,
> > +   sizeof(asma->name + ASHMEM_NAME_PREFIX_LEN));
> 
> There is a parenthesis in the wrong place.

Worse - moving parenthesis to just after asma->name would result in
interestingly bogus value (size + amount skipped instead of size -
amount skipped).

Folks, blind changes in name of security are seriously counterproductive;
fortunately, in this particular case overflow prevention is taken care
of by earlier code (source of strcpy is a local array of size that
isn't enough to cause trouble and it is NUL-terminated), so that
particular strlcpy() is simply pointless, but if not for that...
Variant with sizeof(asma->name) + ASHMEM_NAME_PREFIX_LEN would've
invited an overflow *and* made it harder to spot in the future -
"it uses strlcpy, no worries about overflows here"...

Re: [Outreachy kernel] [PATCH] staging: android: Replace strcpy with strlcpy

[Al Viro]

On Sat, Mar 11, 2017 at 09:47:30PM +0100, Julia Lawall wrote:
> 
> 
> On Sun, 12 Mar 2017, simran singhal wrote:
> 
> > Replace strcpy with strlcpy as strcpy does not check for buffer
> > overflow.
> > This is found using Flawfinder.
> >
> > Signed-off-by: simran singhal 
> > ---
> >  drivers/staging/android/ashmem.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/android/ashmem.c 
> > b/drivers/staging/android/ashmem.c
> > index 7cbad0d..eb2f4ef 100644
> > --- a/drivers/staging/android/ashmem.c
> > +++ b/drivers/staging/android/ashmem.c
> > @@ -548,7 +548,8 @@ static int set_name(struct ashmem_area *asma, void 
> > __user *name)
> > if (unlikely(asma->file))
> > ret = -EINVAL;
> > else
> > -   strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name);
> > +   strlcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name,
> > +   sizeof(asma->name + ASHMEM_NAME_PREFIX_LEN));
> 
> There is a parenthesis in the wrong place.

Worse - moving parenthesis to just after asma->name would result in
interestingly bogus value (size + amount skipped instead of size -
amount skipped).

Folks, blind changes in name of security are seriously counterproductive;
fortunately, in this particular case overflow prevention is taken care
of by earlier code (source of strcpy is a local array of size that
isn't enough to cause trouble and it is NUL-terminated), so that
particular strlcpy() is simply pointless, but if not for that...
Variant with sizeof(asma->name) + ASHMEM_NAME_PREFIX_LEN would've
invited an overflow *and* made it harder to spot in the future -
"it uses strlcpy, no worries about overflows here"...

[PATCH] ASoC: fix semicolon.cocci warnings

[kbuild test robot]

sound/soc/codecs/cs35l35.c:706:2-3: Unneeded semicolon
sound/soc/codecs/cs35l35.c:543:4-5: Unneeded semicolon
sound/soc/codecs/cs35l35.c:553:4-5: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Brian Austin 
Signed-off-by: Fengguang Wu 
---

 cs35l35.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/sound/soc/codecs/cs35l35.c
+++ b/sound/soc/codecs/cs35l35.c
@@ -540,7 +540,7 @@ static int cs35l35_hw_params(struct snd_
default:
dev_err(codec->dev, "ratio not supported\n");
return -EINVAL;
-   };
+   }
} else {
/* Only certain ratios supported in I2S MASTER Mode */
switch (sp_sclks) {
@@ -550,7 +550,7 @@ static int cs35l35_hw_params(struct snd_
default:
dev_err(codec->dev, "ratio not supported\n");
return -EINVAL;
-   };
+   }
}
ret = regmap_update_bits(cs35l35->regmap,
CS35L35_CLK_CTL3,
@@ -703,7 +703,7 @@ static int cs35l35_codec_set_sysclk(stru
default:
dev_err(codec->dev, "Invalid CLK Source\n");
return -EINVAL;
-   };
+   }
 
switch (freq) {
case 5644800:

Re: [PATCH] staging: android: Replace strcpy with strlcpy

[Al Viro]

On Sun, Mar 12, 2017 at 02:10:01AM +0530, simran singhal wrote:
> Replace strcpy with strlcpy as strcpy does not check for buffer
> overflow.
> This is found using Flawfinder.
> 
> Signed-off-by: simran singhal 
> ---
>  drivers/staging/android/ashmem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ashmem.c 
> b/drivers/staging/android/ashmem.c
> index 7cbad0d..eb2f4ef 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -548,7 +548,8 @@ static int set_name(struct ashmem_area *asma, void __user 
> *name)
>   if (unlikely(asma->file))
>   ret = -EINVAL;
>   else
> - strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name);
> + strlcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name,
> + sizeof(asma->name + ASHMEM_NAME_PREFIX_LEN));

Trivial C quiz: given

struct ashmem_area {
char name[ASHMEM_FULL_NAME_LEN];
struct list_head unpinned_list;
struct file *file;
size_t size;
unsigned long prot_mask;
};
static int set_name(struct ashmem_area *asma, void __user *name)

what, in your opinion, would be
1) type of asma->name
2) type of asma->name + ASHMEM_NAME_PREFIX_LEN
3) value of sizeof(asma->name + ASHMEM_NAME_PREFIX_LEN)
As a bonus question,
4) what is the value of this kind of patches?


1) NFUZRZ_SHYY_ANZR_YRA-ryrzrag neenl bs pune
2) cbvagre gb pune
3) fvmr bs n cbvagre
4) fbpvbybtvpny - ernql-znqr vyyhfgengvbaf bs crevyf bs
pnetb phyg.

Re: [PATCH] staging: android: Replace strcpy with strlcpy

[Al Viro]

On Sun, Mar 12, 2017 at 02:10:01AM +0530, simran singhal wrote:
> Replace strcpy with strlcpy as strcpy does not check for buffer
> overflow.
> This is found using Flawfinder.
> 
> Signed-off-by: simran singhal 
> ---
>  drivers/staging/android/ashmem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ashmem.c 
> b/drivers/staging/android/ashmem.c
> index 7cbad0d..eb2f4ef 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -548,7 +548,8 @@ static int set_name(struct ashmem_area *asma, void __user 
> *name)
>   if (unlikely(asma->file))
>   ret = -EINVAL;
>   else
> - strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name);
> + strlcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name,
> + sizeof(asma->name + ASHMEM_NAME_PREFIX_LEN));

Trivial C quiz: given

struct ashmem_area {
char name[ASHMEM_FULL_NAME_LEN];
struct list_head unpinned_list;
struct file *file;
size_t size;
unsigned long prot_mask;
};
static int set_name(struct ashmem_area *asma, void __user *name)

what, in your opinion, would be
1) type of asma->name
2) type of asma->name + ASHMEM_NAME_PREFIX_LEN
3) value of sizeof(asma->name + ASHMEM_NAME_PREFIX_LEN)
As a bonus question,
4) what is the value of this kind of patches?


1) NFUZRZ_SHYY_ANZR_YRA-ryrzrag neenl bs pune
2) cbvagre gb pune
3) fvmr bs n cbvagre
4) fbpvbybtvpny - ernql-znqr vyyhfgengvbaf bs crevyf bs
pnetb phyg.

Re: When will Linux support M2 on RAID ?

[David F.]

Very possible it affects other devices attached, but all consumer
reports and test systems here all have NVME drives on m2 and when in
RAID mode.  Listing PCI data linux will show Intel SATA controller
detected in RAID mode, but no drives detected, all you get is your
/dev/sda USB boot device.  A lot of places linux uses a table of known
ID's and if not listed doesn't support it, unlike Windows which always
supports the given generic device type when it can (keyboard, etc..)
and special drivers for special features.  RAID of course is different
an typically requires special drivers.   As mentioned, some system
don't let you change the mode, others you can't use linux as a
maintenance platform since it won't see any of the drives.  Just be
nice to have things "just work".

On Tue, Mar 7, 2017 at 7:54 AM, Austin S. Hemmelgarn
 wrote:
> On 2017-03-07 10:15, Christoph Hellwig wrote:
>>
>> On Tue, Mar 07, 2017 at 09:50:22AM -0500, Austin S. Hemmelgarn wrote:
>>>
>>> He's referring to the RAID mode most modern Intel chipsets have, which
>>> (last
>>> I checked) Linux does not support completely and many OEM's are setting
>>> by
>>> default on new systems because it apparently provides better performance
>>> than AHCI even for a single device.
>>
>>
>> It actually provides worse performance.  What it does it that it shoves
>> up to three nvme device bars into the bar of an AHCI device, and
>> requires the OS to handle them all using a single driver.  The Money's
>> on crack at Intel decided to do that to provide their "valueable" RSTe
>> IP (which is a windows ATA + RAID driver in a blob, which now has also
>> grown a NVMe driver).  The only remotely sane thing is to disable it
>> in the bios, and burn all people involved with it.  The next best thing
>> is to provide a fake PCIe root port driver untangling this before it
>> hits the driver, but unfortunately Intel is unwilling to either do this
>> on their own or at least provide enough documentation for others to do
>> it.
>>
> For NVMe, yeah, it hurts performance horribly.  For SATA devices though,
> it's hit or miss, some setups perform better, some perform worse.
>
> It does have one advantage though, it lets you put the C drive for a Windows
> install on a soft-RAID array insanely easily compared to trying to do so
> through Windows itself (although still significantly less easily that doing
> the equivalent on Linux...).
>
> The cynic in me is tempted to believe that the OEM's who are turning it on
> by default are trying to either:
> 1. Make their low-end systems look even crappier in terms of performance
> while adding to their marketing checklist (Of the systems I've seen that
> have this on by default, most were cheap ones with really low specs).
> 2. Actively make it harder to run anything but Windows on their hardware.

Re: When will Linux support M2 on RAID ?

[David F.]

Very possible it affects other devices attached, but all consumer
reports and test systems here all have NVME drives on m2 and when in
RAID mode.  Listing PCI data linux will show Intel SATA controller
detected in RAID mode, but no drives detected, all you get is your
/dev/sda USB boot device.  A lot of places linux uses a table of known
ID's and if not listed doesn't support it, unlike Windows which always
supports the given generic device type when it can (keyboard, etc..)
and special drivers for special features.  RAID of course is different
an typically requires special drivers.   As mentioned, some system
don't let you change the mode, others you can't use linux as a
maintenance platform since it won't see any of the drives.  Just be
nice to have things "just work".

On Tue, Mar 7, 2017 at 7:54 AM, Austin S. Hemmelgarn
 wrote:
> On 2017-03-07 10:15, Christoph Hellwig wrote:
>>
>> On Tue, Mar 07, 2017 at 09:50:22AM -0500, Austin S. Hemmelgarn wrote:
>>>
>>> He's referring to the RAID mode most modern Intel chipsets have, which
>>> (last
>>> I checked) Linux does not support completely and many OEM's are setting
>>> by
>>> default on new systems because it apparently provides better performance
>>> than AHCI even for a single device.
>>
>>
>> It actually provides worse performance.  What it does it that it shoves
>> up to three nvme device bars into the bar of an AHCI device, and
>> requires the OS to handle them all using a single driver.  The Money's
>> on crack at Intel decided to do that to provide their "valueable" RSTe
>> IP (which is a windows ATA + RAID driver in a blob, which now has also
>> grown a NVMe driver).  The only remotely sane thing is to disable it
>> in the bios, and burn all people involved with it.  The next best thing
>> is to provide a fake PCIe root port driver untangling this before it
>> hits the driver, but unfortunately Intel is unwilling to either do this
>> on their own or at least provide enough documentation for others to do
>> it.
>>
> For NVMe, yeah, it hurts performance horribly.  For SATA devices though,
> it's hit or miss, some setups perform better, some perform worse.
>
> It does have one advantage though, it lets you put the C drive for a Windows
> install on a soft-RAID array insanely easily compared to trying to do so
> through Windows itself (although still significantly less easily that doing
> the equivalent on Linux...).
>
> The cynic in me is tempted to believe that the OEM's who are turning it on
> by default are trying to either:
> 1. Make their low-end systems look even crappier in terms of performance
> while adding to their marketing checklist (Of the systems I've seen that
> have this on by default, most were cheap ones with really low specs).
> 2. Actively make it harder to run anything but Windows on their hardware.

Re: [PATCH v5 00/39] i.MX Media Driver

[Steve Longerbeam]

On 03/10/2017 12:13 PM, Russell King - ARM Linux wrote:

Version 5 gives me no v4l2 controls exposed through the video device
interface.

Just like with version 4, version 5 is completely useless with IMX219:

imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110



If it's too difficult to get the imx219 csi-2 transmitter into the
LP-11 state on power on, perhaps the csi-2 receiver can be a little
more lenient on the transmitter and make the LP-11 timeout a warning
instead of error-out.

Can you try the attached change on top of the version 5 patchset?

If that doesn't work then you're just going to have to fix the bug
in imx219.

Steve
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index d8f931e..720bf4d 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -224,11 +224,8 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
  (reg & mask) == mask, 0, 50);
-	if (ret) {
-		v4l2_err(>sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
-		return ret;
-	}
-
+	if (ret)
+		v4l2_warn(>sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
 	return 0;
 }
 

Re: [PATCH v5 00/39] i.MX Media Driver

[Steve Longerbeam]

On 03/10/2017 12:13 PM, Russell King - ARM Linux wrote:

Version 5 gives me no v4l2 controls exposed through the video device
interface.

Just like with version 4, version 5 is completely useless with IMX219:

imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0200
ipu1_csi0: pipeline start failed with -110



If it's too difficult to get the imx219 csi-2 transmitter into the
LP-11 state on power on, perhaps the csi-2 receiver can be a little
more lenient on the transmitter and make the LP-11 timeout a warning
instead of error-out.

Can you try the attached change on top of the version 5 patchset?

If that doesn't work then you're just going to have to fix the bug
in imx219.

Steve
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index d8f931e..720bf4d 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -224,11 +224,8 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
  (reg & mask) == mask, 0, 50);
-	if (ret) {
-		v4l2_err(>sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
-		return ret;
-	}
-
+	if (ret)
+		v4l2_warn(>sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
 	return 0;
 }
 

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Steve Longerbeam]

On 03/11/2017 03:14 PM, Russell King - ARM Linux wrote:

On Sat, Mar 11, 2017 at 08:25:49AM -0300, Mauro Carvalho Chehab wrote:

This situation is there since 2009. If I remember well, you tried to write
such generic plugin in the past, but never finished it, apparently because
it is too complex. Others tried too over the years.

The last trial was done by Jacek, trying to cover just the exynos4 driver.
Yet, even such limited scope plugin was not good enough, as it was never
merged upstream. Currently, there's no such plugins upstream.

If we can't even merge a plugin that solves it for just *one* driver,
I have no hope that we'll be able to do it for the generic case.


This is what really worries me right now about the current proposal for
iMX6.  What's being proposed is to make the driver exclusively MC-based.



I don't see anything wrong with that.



What that means is that existing applications are _not_ going to work
until we have some answer for libv4l2, and from what you've said above,
it seems that this has been attempted multiple times over the last _8_
years, and each time it's failed.

When thinking about it, it's quite obvious why merely trying to push
the problem into userspace fails:

  If we assert that the kernel does not have sufficient information to
  make decisions about how to route and control the hardware, then under
  what scenario does a userspace library have sufficient information to
  make those decisions?

So, merely moving the problem into userspace doesn't solve anything.

Loading the problem onto the user in the hope that the user knows
enough to properly configure it also doesn't work - who is going to
educate the user about the various quirks of the hardware they're
dealing with?


Documentation?



I don't think pushing it into platform specific libv4l2 plugins works
either - as you say above, even just trying to develop a plugin for
exynos4 seems to have failed, so what makes us think that developing
a plugin for iMX6 is going to succeed?  Actually, that's exactly where
the problem lies.

Is "iMX6 plugin" even right?  That only deals with the complexity of
one part of the system - what about the source device, which as we
have already seen can be a tuner or a camera with its own multiple
sub-devices.  What if there's a video improvement chip in the chain
as well - how is a "generic" iMX6 plugin supposed to know how to deal
with that?

It seems to me that what's required is not an "iMX6 plugin" but a
separate plugin for each platform - or worse.  Consider boards like
the Raspberry Pi, where users can attach a variety of cameras.  I
don't think this approach scales.  (This is relevant: the iMX6 board
I have here has a RPi compatible connector for a MIPI CSI2 camera.
In fact, the IMX219 module I'm using _is_ a RPi camera, it's the RPi
NoIR Camera V2.)

The iMX6 problem is way larger than just "which subdev do I need to
configure for control X" - if you look at the dot graphs both Steve
and myself have supplied, you'll notice that there are eight (yes,
8) video capture devices.


There are 4 video nodes (per IPU):

- unconverted capture from CSI0
- unconverted capture from CSI1
- scaled, CSC, and/or rotated capture from PRP ENC
- scaled, CSC, rotated, and/or de-interlaced capture from PRP VF


Configuring the imx6 pipelines are not that difficult. I've put quite
a bit of detail in the media doc, so it should become clear to any
user with MC knowledge (even those with absolutely no knowledge of
imx) to quickly start getting working pipelines.



  Let's say that we can solve the subdev

problem in libv4l2.  There's another problem lurking here - libv4l2
is /dev/video* based.  How does it know which /dev/video* device to
open?

We don't open by sensor, we open by /dev/video*.  In my case, there
is only one correct /dev/video* node for the attached sensor, the
other seven are totally irrelevant.  For other situations, there may
be the choice of three functional /dev/video* nodes.

Right now, for my case, there isn't the information exported from the
kernel to know which is the correct one, since that requires knowing
which virtual channel the data is going to be sent over the CSI2
interface.  That information is not present in DT, or anywhere.


It is described in the media doc:

"This is the MIPI CSI-2 receiver entity. It has one sink pad to receive
the MIPI CSI-2 stream (usually from a MIPI CSI-2 camera sensor). It has
four source pads, corresponding to the four MIPI CSI-2 demuxed virtual
channel outputs."



It only comes from system knowledge - in my case, I know that the IMX219
is currently being configured to use virtual channel 0.  SMIA cameras
are also configurable.  Then there's CSI2 cameras that can produce
different formats via different virtual channels (eg, JPEG compressed
image on one channel while streaming a RGB image via the other channel.)

Whether you can use one or three in _this_ scenario depends on the
source format - again, another bit of 

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

[Steve Longerbeam]

On 03/11/2017 03:14 PM, Russell King - ARM Linux wrote:

On Sat, Mar 11, 2017 at 08:25:49AM -0300, Mauro Carvalho Chehab wrote:

This situation is there since 2009. If I remember well, you tried to write
such generic plugin in the past, but never finished it, apparently because
it is too complex. Others tried too over the years.

The last trial was done by Jacek, trying to cover just the exynos4 driver.
Yet, even such limited scope plugin was not good enough, as it was never
merged upstream. Currently, there's no such plugins upstream.

If we can't even merge a plugin that solves it for just *one* driver,
I have no hope that we'll be able to do it for the generic case.


This is what really worries me right now about the current proposal for
iMX6.  What's being proposed is to make the driver exclusively MC-based.



I don't see anything wrong with that.



What that means is that existing applications are _not_ going to work
until we have some answer for libv4l2, and from what you've said above,
it seems that this has been attempted multiple times over the last _8_
years, and each time it's failed.

When thinking about it, it's quite obvious why merely trying to push
the problem into userspace fails:

  If we assert that the kernel does not have sufficient information to
  make decisions about how to route and control the hardware, then under
  what scenario does a userspace library have sufficient information to
  make those decisions?

So, merely moving the problem into userspace doesn't solve anything.

Loading the problem onto the user in the hope that the user knows
enough to properly configure it also doesn't work - who is going to
educate the user about the various quirks of the hardware they're
dealing with?


Documentation?



I don't think pushing it into platform specific libv4l2 plugins works
either - as you say above, even just trying to develop a plugin for
exynos4 seems to have failed, so what makes us think that developing
a plugin for iMX6 is going to succeed?  Actually, that's exactly where
the problem lies.

Is "iMX6 plugin" even right?  That only deals with the complexity of
one part of the system - what about the source device, which as we
have already seen can be a tuner or a camera with its own multiple
sub-devices.  What if there's a video improvement chip in the chain
as well - how is a "generic" iMX6 plugin supposed to know how to deal
with that?

It seems to me that what's required is not an "iMX6 plugin" but a
separate plugin for each platform - or worse.  Consider boards like
the Raspberry Pi, where users can attach a variety of cameras.  I
don't think this approach scales.  (This is relevant: the iMX6 board
I have here has a RPi compatible connector for a MIPI CSI2 camera.
In fact, the IMX219 module I'm using _is_ a RPi camera, it's the RPi
NoIR Camera V2.)

The iMX6 problem is way larger than just "which subdev do I need to
configure for control X" - if you look at the dot graphs both Steve
and myself have supplied, you'll notice that there are eight (yes,
8) video capture devices.


There are 4 video nodes (per IPU):

- unconverted capture from CSI0
- unconverted capture from CSI1
- scaled, CSC, and/or rotated capture from PRP ENC
- scaled, CSC, rotated, and/or de-interlaced capture from PRP VF


Configuring the imx6 pipelines are not that difficult. I've put quite
a bit of detail in the media doc, so it should become clear to any
user with MC knowledge (even those with absolutely no knowledge of
imx) to quickly start getting working pipelines.



  Let's say that we can solve the subdev

problem in libv4l2.  There's another problem lurking here - libv4l2
is /dev/video* based.  How does it know which /dev/video* device to
open?

We don't open by sensor, we open by /dev/video*.  In my case, there
is only one correct /dev/video* node for the attached sensor, the
other seven are totally irrelevant.  For other situations, there may
be the choice of three functional /dev/video* nodes.

Right now, for my case, there isn't the information exported from the
kernel to know which is the correct one, since that requires knowing
which virtual channel the data is going to be sent over the CSI2
interface.  That information is not present in DT, or anywhere.


It is described in the media doc:

"This is the MIPI CSI-2 receiver entity. It has one sink pad to receive
the MIPI CSI-2 stream (usually from a MIPI CSI-2 camera sensor). It has
four source pads, corresponding to the four MIPI CSI-2 demuxed virtual
channel outputs."



It only comes from system knowledge - in my case, I know that the IMX219
is currently being configured to use virtual channel 0.  SMIA cameras
are also configurable.  Then there's CSI2 cameras that can produce
different formats via different virtual channels (eg, JPEG compressed
image on one channel while streaming a RGB image via the other channel.)

Whether you can use one or three in _this_ scenario depends on the
source format - again, another bit of 

Re: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Michael S. Tsirkin]

On Sat, Mar 11, 2017 at 07:59:31PM +0800, Wei Wang wrote:
> On 03/11/2017 01:11 AM, Matthew Wilcox wrote:
> > On Fri, Mar 10, 2017 at 05:58:28PM +0200, Michael S. Tsirkin wrote:
> > > One of the issues of current balloon is the 4k page size
> > > assumption. For example if you free a huge page you
> > > have to split it up and pass 4k chunks to host.
> > > Quite often host can't free these 4k chunks at all (e.g.
> > > when it's using huge tlb fs).
> > > It's even sillier for architectures with base page size >4k.
> > I completely agree with you that we should be able to pass a hugepage
> > as a single chunk.  Also we shouldn't assume that host and guest have
> > the same page size.  I think we can come up with a scheme that actually
> > lets us encode that into a 64-bit word, something like this:
> > 
> > bit 0 clear => bits 1-11 encode a page count, bits 12-63 encode a PFN, page 
> > size 4k.
> > bit 0 set, bit 1 clear => bits 2-12 encode a page count, bits 13-63 encode 
> > a PFN, page size 8k
> > bits 0+1 set, bit 2 clear => bits 3-13 for page count, bits 14-63 for PFN, 
> > page size 16k.
> > bits 0-2 set, bit 3 clear => bits 4-14 for page count, bits 15-63 for PFN, 
> > page size 32k
> > bits 0-3 set, bit 4 clear => bits 5-15 for page count, bits 16-63 for PFN, 
> > page size 64k
> > 
> > That means we can always pass 2048 pages (of whatever page size) in a 
> > single chunk.  And
> > we support arbitrary power of two page sizes.  I suggest something like 
> > this:
> > 
> > u64 page_to_chunk(struct page *page)
> > {
> > u64 chunk = page_to_pfn(page) << PAGE_SHIFT;
> > chunk |= (1UL << compound_order(page)) - 1;
> > }
> > 
> > (note this is a single page of order N, so we leave the page count bits
> > set to 0, meaning one page).
> > 
> 
> I'm thinking what if the guest needs to transfer these much physically
> continuous
> memory to host: 1GB+2MB+64KB+32KB+16KB+4KB.
> Is it going to use Six 64-bit chunks? Would it be simpler if we just
> use the 128-bit chunk format (we can drop the previous normal 64-bit
> format)?
> 
> Best,
> Wei

I think I prefer that as a more straightforward approach, but
I can live with either approach.


-- 
MST

Re: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Michael S. Tsirkin]

On Sat, Mar 11, 2017 at 07:59:31PM +0800, Wei Wang wrote:
> On 03/11/2017 01:11 AM, Matthew Wilcox wrote:
> > On Fri, Mar 10, 2017 at 05:58:28PM +0200, Michael S. Tsirkin wrote:
> > > One of the issues of current balloon is the 4k page size
> > > assumption. For example if you free a huge page you
> > > have to split it up and pass 4k chunks to host.
> > > Quite often host can't free these 4k chunks at all (e.g.
> > > when it's using huge tlb fs).
> > > It's even sillier for architectures with base page size >4k.
> > I completely agree with you that we should be able to pass a hugepage
> > as a single chunk.  Also we shouldn't assume that host and guest have
> > the same page size.  I think we can come up with a scheme that actually
> > lets us encode that into a 64-bit word, something like this:
> > 
> > bit 0 clear => bits 1-11 encode a page count, bits 12-63 encode a PFN, page 
> > size 4k.
> > bit 0 set, bit 1 clear => bits 2-12 encode a page count, bits 13-63 encode 
> > a PFN, page size 8k
> > bits 0+1 set, bit 2 clear => bits 3-13 for page count, bits 14-63 for PFN, 
> > page size 16k.
> > bits 0-2 set, bit 3 clear => bits 4-14 for page count, bits 15-63 for PFN, 
> > page size 32k
> > bits 0-3 set, bit 4 clear => bits 5-15 for page count, bits 16-63 for PFN, 
> > page size 64k
> > 
> > That means we can always pass 2048 pages (of whatever page size) in a 
> > single chunk.  And
> > we support arbitrary power of two page sizes.  I suggest something like 
> > this:
> > 
> > u64 page_to_chunk(struct page *page)
> > {
> > u64 chunk = page_to_pfn(page) << PAGE_SHIFT;
> > chunk |= (1UL << compound_order(page)) - 1;
> > }
> > 
> > (note this is a single page of order N, so we leave the page count bits
> > set to 0, meaning one page).
> > 
> 
> I'm thinking what if the guest needs to transfer these much physically
> continuous
> memory to host: 1GB+2MB+64KB+32KB+16KB+4KB.
> Is it going to use Six 64-bit chunks? Would it be simpler if we just
> use the 128-bit chunk format (we can drop the previous normal 64-bit
> format)?
> 
> Best,
> Wei

I think I prefer that as a more straightforward approach, but
I can live with either approach.


-- 
MST

Re: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Michael S. Tsirkin]

On Fri, Mar 10, 2017 at 01:25:41PM -0800, Matthew Wilcox wrote:
> On Fri, Mar 10, 2017 at 09:35:21PM +0200, Michael S. Tsirkin wrote:
> > > bit 0 clear => bits 1-11 encode a page count, bits 12-63 encode a PFN, 
> > > page size 4k.
> > > bit 0 set, bit 1 clear => bits 2-12 encode a page count, bits 13-63 
> > > encode a PFN, page size 8k
> > > bits 0+1 set, bit 2 clear => bits 3-13 for page count, bits 14-63 for 
> > > PFN, page size 16k.
> > > bits 0-2 set, bit 3 clear => bits 4-14 for page count, bits 15-63 for 
> > > PFN, page size 32k
> > > bits 0-3 set, bit 4 clear => bits 5-15 for page count, bits 16-63 for 
> > > PFN, page size 64k
> > > That means we can always pass 2048 pages (of whatever page size) in a 
> > > single chunk.  And
> > > we support arbitrary power of two page sizes.  I suggest something like 
> > > this:
> > > 
> > > u64 page_to_chunk(struct page *page)
> > > {
> > >   u64 chunk = page_to_pfn(page) << PAGE_SHIFT;
> > >   chunk |= (1UL << compound_order(page)) - 1;
> > > }
> > 
> > You need to fill in the size, do you not?
> 
> I think I did ... (1UL << compound_order(page)) - 1 sets the bottom
> N bits.  Bit N+1 will already be clear.  What am I missing?

This sets the order but not the number of pages.
For that you would do something like

chunk |= size << compound_order(page)

right?

> > > > - host should pass its base page size to guest
> > > >   this can be a separate patch and for now we can fall back on 12 bit 
> > > > if not there
> > > 
> > > With this encoding scheme, I don't think we need to do this?  As long as
> > > it's *at least* 12 bit, then we're fine.
> > 
> > I think we will still need something like this down the road.  The point
> > is that not all hosts are able to use 4k pages in a balloon.
> > So it's pointless for guest to pass 4k pages to such a host,
> > and we need host to tell guest the page size it needs.
> > 
> > However that's a separate feature that can wait until
> > another day.
> 
> Ah, the TRIM/DISCARD debate all over again ... should the guest batch
> up or should the host do that work ... probably easier to account it in
> the guest.  Might be better to frame it as 'balloon chunk size' rather than
> host page size as it might have nothing to do with the host page size.

Exactly.

> > > What per-chunk flags are you thinking would be useful?
> > 
> > Not entirely sure but I think would have been prudent to leave some free
> > if possible. Your encoding seems to use them all up, so be it.
> 
> We don't necessarily have to support 2048 pages in a single chunk.
> If it's worth reserving some bits, we can do that at the expense of
> reducing the maximum number of pages per chunk.

Well we can always change things with a feature bit ..
I'll leave this up to you and Wei.

-- 
MST

Re: [PATCH v7 kernel 3/5] virtio-balloon: implementation of VIRTIO_BALLOON_F_CHUNK_TRANSFER

[Michael S. Tsirkin]

On Fri, Mar 10, 2017 at 01:25:41PM -0800, Matthew Wilcox wrote:
> On Fri, Mar 10, 2017 at 09:35:21PM +0200, Michael S. Tsirkin wrote:
> > > bit 0 clear => bits 1-11 encode a page count, bits 12-63 encode a PFN, 
> > > page size 4k.
> > > bit 0 set, bit 1 clear => bits 2-12 encode a page count, bits 13-63 
> > > encode a PFN, page size 8k
> > > bits 0+1 set, bit 2 clear => bits 3-13 for page count, bits 14-63 for 
> > > PFN, page size 16k.
> > > bits 0-2 set, bit 3 clear => bits 4-14 for page count, bits 15-63 for 
> > > PFN, page size 32k
> > > bits 0-3 set, bit 4 clear => bits 5-15 for page count, bits 16-63 for 
> > > PFN, page size 64k
> > > That means we can always pass 2048 pages (of whatever page size) in a 
> > > single chunk.  And
> > > we support arbitrary power of two page sizes.  I suggest something like 
> > > this:
> > > 
> > > u64 page_to_chunk(struct page *page)
> > > {
> > >   u64 chunk = page_to_pfn(page) << PAGE_SHIFT;
> > >   chunk |= (1UL << compound_order(page)) - 1;
> > > }
> > 
> > You need to fill in the size, do you not?
> 
> I think I did ... (1UL << compound_order(page)) - 1 sets the bottom
> N bits.  Bit N+1 will already be clear.  What am I missing?

This sets the order but not the number of pages.
For that you would do something like

chunk |= size << compound_order(page)

right?

> > > > - host should pass its base page size to guest
> > > >   this can be a separate patch and for now we can fall back on 12 bit 
> > > > if not there
> > > 
> > > With this encoding scheme, I don't think we need to do this?  As long as
> > > it's *at least* 12 bit, then we're fine.
> > 
> > I think we will still need something like this down the road.  The point
> > is that not all hosts are able to use 4k pages in a balloon.
> > So it's pointless for guest to pass 4k pages to such a host,
> > and we need host to tell guest the page size it needs.
> > 
> > However that's a separate feature that can wait until
> > another day.
> 
> Ah, the TRIM/DISCARD debate all over again ... should the guest batch
> up or should the host do that work ... probably easier to account it in
> the guest.  Might be better to frame it as 'balloon chunk size' rather than
> host page size as it might have nothing to do with the host page size.

Exactly.

> > > What per-chunk flags are you thinking would be useful?
> > 
> > Not entirely sure but I think would have been prudent to leave some free
> > if possible. Your encoding seems to use them all up, so be it.
> 
> We don't necessarily have to support 2048 pages in a single chunk.
> If it's worth reserving some bits, we can do that at the expense of
> reducing the maximum number of pages per chunk.

Well we can always change things with a feature bit ..
I'll leave this up to you and Wei.

-- 
MST

Re: [PATCH] kvm: better MWAIT emulation for guests

[Michael S. Tsirkin]

On Fri, Mar 10, 2017 at 03:46:45PM -0800, Jim Mattson wrote:
> On Thu, Mar 9, 2017 at 2:29 PM, Michael S. Tsirkin  wrote:
> > Some guests call mwait without checking the cpu flags.  We currently
> 
> "Some guests"? What guests other than Mac OS X are so ill-behaved?

I heard about Mac OSX only but even that is hearsay for me
so I didn't want to say that explicitly.

> > emulate that as a NOP but on VMX we can do better: let guest stop the
> > CPU until timer or IPI.  CPU will be busy but that isn't any worse than
> > a NOP emulation.
> >
> > Note that mwait within guests is not the same as on real hardware
> > because you must halt if you want to go deep into sleep.  Thus it isn't
> > a good idea to use the regular MWAIT flag in CPUID for that.  Add a flag
> > in the hypervisor leaf instead.
> >
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  Documentation/virtual/kvm/cpuid.txt  | 3 +++
> >  arch/x86/include/uapi/asm/kvm_para.h | 1 +
> >  arch/x86/kvm/cpuid.c | 3 +++
> >  arch/x86/kvm/vmx.c   | 4 
> >  4 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/virtual/kvm/cpuid.txt 
> > b/Documentation/virtual/kvm/cpuid.txt
> > index 3c65feb..5caa234 100644
> > --- a/Documentation/virtual/kvm/cpuid.txt
> > +++ b/Documentation/virtual/kvm/cpuid.txt
> > @@ -54,6 +54,9 @@ KVM_FEATURE_PV_UNHALT  || 7 || guest 
> > checks this feature bit
> > ||   || before enabling 
> > paravirtualized
> > ||   || spinlock support.
> >  
> > --
> > +KVM_FEATURE_MWAIT  || 8 || guest can use monitor/mwait
> > +   ||   || to halt the VCPU.
> > +--
> >  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||24 || host will warn if no 
> > guest-side
> > ||   || per-cpu warps are expected 
> > in
> > ||   || kvmclock.
> > diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
> > b/arch/x86/include/uapi/asm/kvm_para.h
> > index cff0bb6..9cc77a7 100644
> > --- a/arch/x86/include/uapi/asm/kvm_para.h
> > +++ b/arch/x86/include/uapi/asm/kvm_para.h
> > @@ -24,6 +24,7 @@
> >  #define KVM_FEATURE_STEAL_TIME 5
> >  #define KVM_FEATURE_PV_EOI 6
> >  #define KVM_FEATURE_PV_UNHALT  7
> > +#define KVM_FEATURE_MWAIT  8
> >
> >  /* The last 8 bits are used to indicate how to interpret the flags field
> >   * in pvclock structure. If no bits are set, all flags are ignored.
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index efde6cc..fe3d292 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -594,6 +594,9 @@ static inline int __do_cpuid_ent(struct 
> > kvm_cpuid_entry2 *entry, u32 function,
> > if (sched_info_on())
> > entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
> >
> > +   if (this_cpu_has(X86_FEATURE_MWAIT))
> > +   entry->eax = (1 << KVM_FEATURE_MWAIT);
> > +
> > entry->ebx = 0;
> > entry->ecx = 0;
> > entry->edx = 0;
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 4bfe349..b167aba 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -3547,13 +3547,9 @@ static __init int setup_vmcs_config(struct 
> > vmcs_config *vmcs_conf)
> >   CPU_BASED_USE_IO_BITMAPS |
> >   CPU_BASED_MOV_DR_EXITING |
> >   CPU_BASED_USE_TSC_OFFSETING |
> > - CPU_BASED_MWAIT_EXITING |
> > - CPU_BASED_MONITOR_EXITING |
> >   CPU_BASED_INVLPG_EXITING |
> >   CPU_BASED_RDPMC_EXITING;
> >
> > -   printk(KERN_ERR "cleared CPU_BASED_MWAIT_EXITING + 
> > CPU_BASED_MONITOR_EXITING\n");
> > -
> > opt = CPU_BASED_TPR_SHADOW |
> >   CPU_BASED_USE_MSR_BITMAPS |
> >   CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
> > --
> > MST

Re: [PATCH] kvm: better MWAIT emulation for guests

[Michael S. Tsirkin]

On Fri, Mar 10, 2017 at 03:46:45PM -0800, Jim Mattson wrote:
> On Thu, Mar 9, 2017 at 2:29 PM, Michael S. Tsirkin  wrote:
> > Some guests call mwait without checking the cpu flags.  We currently
> 
> "Some guests"? What guests other than Mac OS X are so ill-behaved?

I heard about Mac OSX only but even that is hearsay for me
so I didn't want to say that explicitly.

> > emulate that as a NOP but on VMX we can do better: let guest stop the
> > CPU until timer or IPI.  CPU will be busy but that isn't any worse than
> > a NOP emulation.
> >
> > Note that mwait within guests is not the same as on real hardware
> > because you must halt if you want to go deep into sleep.  Thus it isn't
> > a good idea to use the regular MWAIT flag in CPUID for that.  Add a flag
> > in the hypervisor leaf instead.
> >
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  Documentation/virtual/kvm/cpuid.txt  | 3 +++
> >  arch/x86/include/uapi/asm/kvm_para.h | 1 +
> >  arch/x86/kvm/cpuid.c | 3 +++
> >  arch/x86/kvm/vmx.c   | 4 
> >  4 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/virtual/kvm/cpuid.txt 
> > b/Documentation/virtual/kvm/cpuid.txt
> > index 3c65feb..5caa234 100644
> > --- a/Documentation/virtual/kvm/cpuid.txt
> > +++ b/Documentation/virtual/kvm/cpuid.txt
> > @@ -54,6 +54,9 @@ KVM_FEATURE_PV_UNHALT  || 7 || guest 
> > checks this feature bit
> > ||   || before enabling 
> > paravirtualized
> > ||   || spinlock support.
> >  
> > --
> > +KVM_FEATURE_MWAIT  || 8 || guest can use monitor/mwait
> > +   ||   || to halt the VCPU.
> > +--
> >  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||24 || host will warn if no 
> > guest-side
> > ||   || per-cpu warps are expected 
> > in
> > ||   || kvmclock.
> > diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
> > b/arch/x86/include/uapi/asm/kvm_para.h
> > index cff0bb6..9cc77a7 100644
> > --- a/arch/x86/include/uapi/asm/kvm_para.h
> > +++ b/arch/x86/include/uapi/asm/kvm_para.h
> > @@ -24,6 +24,7 @@
> >  #define KVM_FEATURE_STEAL_TIME 5
> >  #define KVM_FEATURE_PV_EOI 6
> >  #define KVM_FEATURE_PV_UNHALT  7
> > +#define KVM_FEATURE_MWAIT  8
> >
> >  /* The last 8 bits are used to indicate how to interpret the flags field
> >   * in pvclock structure. If no bits are set, all flags are ignored.
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index efde6cc..fe3d292 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -594,6 +594,9 @@ static inline int __do_cpuid_ent(struct 
> > kvm_cpuid_entry2 *entry, u32 function,
> > if (sched_info_on())
> > entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
> >
> > +   if (this_cpu_has(X86_FEATURE_MWAIT))
> > +   entry->eax = (1 << KVM_FEATURE_MWAIT);
> > +
> > entry->ebx = 0;
> > entry->ecx = 0;
> > entry->edx = 0;
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 4bfe349..b167aba 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -3547,13 +3547,9 @@ static __init int setup_vmcs_config(struct 
> > vmcs_config *vmcs_conf)
> >   CPU_BASED_USE_IO_BITMAPS |
> >   CPU_BASED_MOV_DR_EXITING |
> >   CPU_BASED_USE_TSC_OFFSETING |
> > - CPU_BASED_MWAIT_EXITING |
> > - CPU_BASED_MONITOR_EXITING |
> >   CPU_BASED_INVLPG_EXITING |
> >   CPU_BASED_RDPMC_EXITING;
> >
> > -   printk(KERN_ERR "cleared CPU_BASED_MWAIT_EXITING + 
> > CPU_BASED_MONITOR_EXITING\n");
> > -
> > opt = CPU_BASED_TPR_SHADOW |
> >   CPU_BASED_USE_MSR_BITMAPS |
> >   CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
> > --
> > MST

[PATCH] kbuild/mkspec: Fix architectures where KBUILD_IMAGE isn't a full path

[Tom Rini]

On some architectures, such as arm64, KBUILD_IMAGE is not a full path
but instead just the build target.  The builddeb script handles this
case correctly today and will try arch/$ARCH/boot/$KBUILD_IMAGE so we
can just borrow that logic and adapt it slightly for spec file syntax.

Cc: Michal Marek 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Catalin Marinas 
Cc: Will Deacon 
Signed-off-by: Tom Rini 
---
It is currently a mixed-bag on if architectures will use a build target
(arm, arm64, arc are certainly by inspection and a few others 'may') or
a full path (x86, blackfin, s390).  Given that builddeb gets this case
correct, I think changing mkspec is the right way to go here.
---
 scripts/package/mkspec | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/package/mkspec b/scripts/package/mkspec
index bb43f153fd8e..aa5f3db43f12 100755
--- a/scripts/package/mkspec
+++ b/scripts/package/mkspec
@@ -101,7 +101,11 @@ echo "%ifarch ppc64"
 echo "cp vmlinux arch/powerpc/boot"
 echo "cp arch/powerpc/boot/"'$KBUILD_IMAGE 
$RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
 echo "%else"
-echo 'cp $KBUILD_IMAGE $RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
+echo "if [ -e $KBUILD_IMAGE ]; then"
+echo '  cp $KBUILD_IMAGE $RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
+echo "else"
+echo '  cp arch/$ARCH/boot/$KBUILD_IMAGE 
$RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
+echo "fi"
 echo "%endif"
 echo "%endif"
 
-- 
2.7.4

[PATCH] kbuild/mkspec: Fix architectures where KBUILD_IMAGE isn't a full path

[Tom Rini]

On some architectures, such as arm64, KBUILD_IMAGE is not a full path
but instead just the build target.  The builddeb script handles this
case correctly today and will try arch/$ARCH/boot/$KBUILD_IMAGE so we
can just borrow that logic and adapt it slightly for spec file syntax.

Cc: Michal Marek 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Catalin Marinas 
Cc: Will Deacon 
Signed-off-by: Tom Rini 
---
It is currently a mixed-bag on if architectures will use a build target
(arm, arm64, arc are certainly by inspection and a few others 'may') or
a full path (x86, blackfin, s390).  Given that builddeb gets this case
correct, I think changing mkspec is the right way to go here.
---
 scripts/package/mkspec | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/package/mkspec b/scripts/package/mkspec
index bb43f153fd8e..aa5f3db43f12 100755
--- a/scripts/package/mkspec
+++ b/scripts/package/mkspec
@@ -101,7 +101,11 @@ echo "%ifarch ppc64"
 echo "cp vmlinux arch/powerpc/boot"
 echo "cp arch/powerpc/boot/"'$KBUILD_IMAGE 
$RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
 echo "%else"
-echo 'cp $KBUILD_IMAGE $RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
+echo "if [ -e $KBUILD_IMAGE ]; then"
+echo '  cp $KBUILD_IMAGE $RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
+echo "else"
+echo '  cp arch/$ARCH/boot/$KBUILD_IMAGE 
$RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
+echo "fi"
 echo "%endif"
 echo "%endif"
 
-- 
2.7.4

Re: [PATCH net-next v2 03/17] net: dsa: mv88e6xxx: add ATU setup helper

[Andrew Lunn]

On Sat, Mar 11, 2017 at 04:12:49PM -0500, Vivien Didelot wrote:
> Move the configuration of the default ageing time in a new
> mv88e6xxx_atu_setup function.
> 
> That function will be extended later to contain all ATU related
> configuration bits.
> 
> Signed-off-by: Vivien Didelot 

Reviewed-by: Andrew Lunn 

Andrew

Re: [PATCH net-next v2 03/17] net: dsa: mv88e6xxx: add ATU setup helper

[Andrew Lunn]

On Sat, Mar 11, 2017 at 04:12:49PM -0500, Vivien Didelot wrote:
> Move the configuration of the default ageing time in a new
> mv88e6xxx_atu_setup function.
> 
> That function will be extended later to contain all ATU related
> configuration bits.
> 
> Signed-off-by: Vivien Didelot 

Reviewed-by: Andrew Lunn 

Andrew

Re: [PATCH net-next v2 17/17] etherdevice: remove unused eth_addr_greater

[Andrew Lunn]

On Sat, Mar 11, 2017 at 04:13:03PM -0500, Vivien Didelot wrote:
> eth_addr_greater() was introduced for the mv88e6xxx driver, but is not
> used anymore. There is no other user, thus remove this function.
> 
> Signed-off-by: Vivien Didelot 

Reviewed-by: Andrew Lunn 

Andrew

Re: [PATCH 3.16 125/370] clk: renesas: mstp: Support 8-bit registers for r7s72100

[Ben Hutchings]

On Fri, 2017-03-10 at 13:10 +0100, Geert Uytterhoeven wrote:
> Hi Ben,
> 
> > On Fri, Mar 10, 2017 at 12:46 PM, Ben Hutchings  
> > wrote:
> > 3.16.42-rc1 review patch.  If anyone has any objections, please let me know.
> 
> No objections, but you also want
> 
> commit f59de563358eb9351b7f8f0ba2d3be2ebb70b93d
> Author: Chris Brandt 
> Date:   Tue Feb 14 11:08:05 2017 -0500
> 
> clk: renesas: mstp: ensure register writes complete
[...]

I've only looked for 'cc: stable' and 'fixes:' in commits up to 4.10 so
far.  Since that commit went into 4.11-rc1, I haven't overlooked it but
will pick it up in the next batch.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.



signature.asc
Description: This is a digitally signed message part

Re: [PATCH net-next v2 17/17] etherdevice: remove unused eth_addr_greater

[Andrew Lunn]

On Sat, Mar 11, 2017 at 04:13:03PM -0500, Vivien Didelot wrote:
> eth_addr_greater() was introduced for the mv88e6xxx driver, but is not
> used anymore. There is no other user, thus remove this function.
> 
> Signed-off-by: Vivien Didelot 

Reviewed-by: Andrew Lunn 

Andrew

Re: [PATCH 3.16 125/370] clk: renesas: mstp: Support 8-bit registers for r7s72100

[Ben Hutchings]

On Fri, 2017-03-10 at 13:10 +0100, Geert Uytterhoeven wrote:
> Hi Ben,
> 
> > On Fri, Mar 10, 2017 at 12:46 PM, Ben Hutchings  
> > wrote:
> > 3.16.42-rc1 review patch.  If anyone has any objections, please let me know.
> 
> No objections, but you also want
> 
> commit f59de563358eb9351b7f8f0ba2d3be2ebb70b93d
> Author: Chris Brandt 
> Date:   Tue Feb 14 11:08:05 2017 -0500
> 
> clk: renesas: mstp: ensure register writes complete
[...]

I've only looked for 'cc: stable' and 'fixes:' in commits up to 4.10 so
far.  Since that commit went into 4.11-rc1, I haven't overlooked it but
will pick it up in the next batch.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.



signature.asc
Description: This is a digitally signed message part