Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[Pankaj Gupta]

Hi David,

> 
> Mark inflated and never onlined pages PG_offline, to tell the world that
> the content is stale and should not be dumped.
> 
> Cc: "K. Y. Srinivasan" 
> Cc: Haiyang Zhang 
> Cc: Stephen Hemminger 
> Cc: Kairui Song 
> Cc: Vitaly Kuznetsov 
> Cc: Andrew Morton 
> Cc: Matthew Wilcox 
> Cc: Michal Hocko 
> Cc: "Michael S. Tsirkin" 
> Signed-off-by: David Hildenbrand 
> ---
>  drivers/hv/hv_balloon.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 211f3fe3a038..47719862e57f 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
>  /* Check if the particular page is backed and can be onlined and online it.
>  */
>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
>  {
> - if (!has_pfn_is_backed(has, page_to_pfn(pg)))
> + if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
> + if (!PageOffline(pg))
> + __SetPageOffline(pg);
>   return;
> + }
> + if (PageOffline(pg))
> + __ClearPageOffline(pg);
>  
>   /* This frame is currently backed; online the page. */
>   __online_page_set_limits(pg);
> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct hv_dynmem_device
> *dm,
>  
>   for (i = 0; i < num_pages; i++) {
>   pg = pfn_to_page(i + start_frame);
> + __ClearPageOffline(pg);

Just thinking, do we need to care for clearing PageOffline flag before freeing
a balloon'd page?

Thanks,
Pankaj

>   __free_page(pg);
>   dm->num_pages_ballooned--;
>   }
> @@ -1213,7 +1219,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
>   struct dm_balloon_response *bl_resp,
>   int alloc_unit)
>  {
> - unsigned int i = 0;
> + unsigned int i, j;
>   struct page *pg;
>  
>   if (num_pages < alloc_unit)
> @@ -1245,6 +1251,10 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
>   if (alloc_unit != 1)
>   split_page(pg, get_order(alloc_unit << PAGE_SHIFT));
>  
> + /* mark all pages offline */
> + for (j = 0; j < (1 << get_order(alloc_unit << PAGE_SHIFT)); j++)
> + __SetPageOffline(pg + j);
> +
>   bl_resp->range_count++;
>   bl_resp->range_array[i].finfo.start_page =
>   page_to_pfn(pg);
> --
> 2.17.2
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[David Hildenbrand]

On 20.11.18 09:45, Pankaj Gupta wrote:
> 
> Hi David,
> 
>>
>> Mark inflated and never onlined pages PG_offline, to tell the world that
>> the content is stale and should not be dumped.
>>
>> Cc: "K. Y. Srinivasan" 
>> Cc: Haiyang Zhang 
>> Cc: Stephen Hemminger 
>> Cc: Kairui Song 
>> Cc: Vitaly Kuznetsov 
>> Cc: Andrew Morton 
>> Cc: Matthew Wilcox 
>> Cc: Michal Hocko 
>> Cc: "Michael S. Tsirkin" 
>> Signed-off-by: David Hildenbrand 
>> ---
>>  drivers/hv/hv_balloon.c | 14 --
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>> index 211f3fe3a038..47719862e57f 100644
>> --- a/drivers/hv/hv_balloon.c
>> +++ b/drivers/hv/hv_balloon.c
>> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
>>  /* Check if the particular page is backed and can be onlined and online it.
>>  */
>>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
>>  {
>> -if (!has_pfn_is_backed(has, page_to_pfn(pg)))
>> +if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
>> +if (!PageOffline(pg))
>> +__SetPageOffline(pg);
>>  return;
>> +}
>> +if (PageOffline(pg))
>> +__ClearPageOffline(pg);
>>  
>>  /* This frame is currently backed; online the page. */
>>  __online_page_set_limits(pg);
>> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct hv_dynmem_device
>> *dm,
>>  
>>  for (i = 0; i < num_pages; i++) {
>>  pg = pfn_to_page(i + start_frame);
>> +__ClearPageOffline(pg);
> 
> Just thinking, do we need to care for clearing PageOffline flag before freeing
> a balloon'd page?

Yes we have to otherwise the code will crash when trying to set PageBuddy.

(only one page type at a time may be set right now, and it makes sense.
A page that is offline cannot e.g. be a buddy page)

So PageOffline is completely managed by the page owner.

> 
> Thanks,
> Pankaj


-- 

Thanks,

David / dhildenb
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[Pankaj Gupta]

> >> ---
> >>  drivers/hv/hv_balloon.c | 14 --
> >>  1 file changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> >> index 211f3fe3a038..47719862e57f 100644
> >> --- a/drivers/hv/hv_balloon.c
> >> +++ b/drivers/hv/hv_balloon.c
> >> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
> >>  /* Check if the particular page is backed and can be onlined and online
> >>  it.
> >>  */
> >>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page
> >>  *pg)
> >>  {
> >> -  if (!has_pfn_is_backed(has, page_to_pfn(pg)))
> >> +  if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
> >> +  if (!PageOffline(pg))
> >> +  __SetPageOffline(pg);
> >>return;
> >> +  }
> >> +  if (PageOffline(pg))
> >> +  __ClearPageOffline(pg);
> >>  
> >>/* This frame is currently backed; online the page. */
> >>__online_page_set_limits(pg);
> >> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct
> >> hv_dynmem_device
> >> *dm,
> >>  
> >>for (i = 0; i < num_pages; i++) {
> >>pg = pfn_to_page(i + start_frame);
> >> +  __ClearPageOffline(pg);
> > 
> > Just thinking, do we need to care for clearing PageOffline flag before
> > freeing
> > a balloon'd page?
> 
> Yes we have to otherwise the code will crash when trying to set PageBuddy.
> 
> (only one page type at a time may be set right now, and it makes sense.
> A page that is offline cannot e.g. be a buddy page)

o.k
> 
> So PageOffline is completely managed by the page owner.

Makes sense. Thanks for explaining.

Pankaj
> 
> > 
> > Thanks,
> > Pankaj
> 
> 
> --
> 
> Thanks,
> 
> David / dhildenb
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging : Add RIFFA PCIe driver

[gre...@linuxfoundation.org]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

http://daringfireball.net/2007/07/on_top

On Sun, Nov 18, 2018 at 01:46:52PM +, Cheng Fei Phung wrote:
> Hi,
> 
> if I only have one particular hardware (Altera DE4 FPGA) that I could test 
> with (there are a bunch of hardware boards that RIFFA supports as you can see 
> from https://github.com/KastnerRG/riffa/blob/master/fpga/altera/Makefile#L42 
> and 
> https://github.com/KastnerRG/riffa/blob/master/fpga/xilinx/Makefile#L42-L43 
> ), what could I do in this case ?

Just use the board you can test with for now, let others add their
device ids as they test.

> I do not understand what you exactly mean by "72 columns" ?

Your email was written all in one line, as above, and you did not wrap
your columns at 72 characters.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: greybus: arche-platform: Switch to the gpio descriptor interface

[Greg Kroah-Hartman]

On Sat, Nov 17, 2018 at 04:40:27PM +0100, Johan Hovold wrote:
> On Sat, Nov 17, 2018 at 12:41:11PM +0530, Nishad Kamdar wrote:
> > On Fri, Nov 16, 2018 at 05:06:22PM +0100, Johan Hovold wrote:
> > > On Fri, Nov 16, 2018 at 08:47:44PM +0530, Nishad Kamdar wrote:
> > > > Use the gpiod interface instead of the deprecated
> > > > old non-descriptor interface.
> > > > 
> > > > Signed-off-by: Nishad Kamdar 
> > > > ---
> > > 
> > > Always include a change log here after the cut-off line so we know what
> > > changed since previous versions.
> > > 
> > > Also include the patch revision in the Subject (e.g. "[PATCH v3]
> > > staging: greybus: ...").
> > > 
> > 
> > Ok, but this is the first patch version that I submitted
> > for greybus: arche-platform.
> 
> Ah, sorry. I thought this was an update of the arche-apb-ctrl patch.

Me too.

I am totally confused as to what is, and is not, the latest versions of
all of these patches :(

Nishad, can you resend all of your pending greybus patches, as a patch
series, as "v3" so that I know which to properly review?  I've dropped
all of your others from my review queue now.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8188eu: core: coding style check

[Greg Kroah-Hartman]

On Tue, Nov 13, 2018 at 10:06:13PM +0800, Kevin Dou wrote:
> follow the linux coding style, rename the variable
> shortGIrate to short_gi_rate.
> 
> Signed-off-by: Kevin Dou 
> ---
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] driver-staging: vsoc.c: Add sysfs support for examining the permissions of regions.

[Greg Kroah-Hartman]

On Tue, Nov 13, 2018 at 11:32:51AM +0800, wahahab wrote:
> 
> > On 12 Nov 2018, at 8:40 PM, Greg Kroah-Hartman  wrote:
> > 
> > On Mon, Nov 12, 2018 at 04:49:30PM +0800, wahahab wrote:
> >> 
> >>> On 10 Nov 2018, at 1:15 AM, Dan Carpenter  >>> > wrote:
> >>> 
> >>> On Wed, Nov 07, 2018 at 10:30:43AM +0800, Jerry Lin wrote:
>  Add a attribute called permissions under vsoc device node for examining
>  current granted permissions in vsoc_device.
>  
>  This file will display permissions in following format:
>  begin_offset  end_offset  owner_offset  owned_value
>   %x  %x%x   %x
>  
> >>> 
> >>> (I'm not totally an expert on sysfs rules).
> >>> 
> >>> Sysfs are supposed to be one value per file, so instead of doing this
> >>> you would create a directory with four files like
> >>> vsoc_device/begin_offset vsoc_device/end_offset etc.  And each would
> >>> just hae a %x output.
> >> 
> >> Thanks for your advice. I have started with this approach.
> >> 
> >> But when I trying to create this kind of sysfs hierarchy. I encountered 
> >> the difficulties of file organizing.
> >> 
> >> My current thought is to create a folder under the device node called 
> >> permissions and user can examine
> >> permission though file path like 
> >> vsoc_device/permissions/permission1/begin_offset. But there comes a
> >> problem that it seems hard to determine the correct index of permission to 
> >> make folder name unique.
> >> 
> >> The solution I come up with is to use memory address of permission node to 
> >> be the index of permission.
> >> So the path will be something like 
> >> vsoc_device/permissions/0x4d23f/begin_offset.
> >> Is this OK for sysfs?
> > 
> > Ick, that is messy.  What exactly are you trying to export and why use
> > sysfs?  Is this just debugging information?  Who is going to use this
> > data?
> 
> I felt that exporting these information in sysfs will add lots of 
> complexities in this driver.
> And I’m not sure these informations are for user space or just for debugging.
> 
> It seems there is a conflict of TODO messages between TODO file and the
> comment in vsoc.c.
> 
> Should I use debugfs first for this patch?

If it is for debugging, yes.  As I have no idea what this code is doing,
or what wants that information, it is hard to determine, sorry.

good luck!

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 00/16] staging: vchiq: dead code removal & misc fixes

[Greg KH]

On Sun, Nov 18, 2018 at 04:55:49PM +0100, Stefan Wahren wrote:
> Hi Nicolas,
> 
> > Nicolas Saenz Julienne  hat am 14. November 2018 um 
> > 13:59 geschrieben:
> > 
> > 
> > Hi All,
> > 
> > This series was written in parallel with reading and understanding the
> > vchiq code. So excuse me for the lack of logic in the sequence of
> > patches.
> > 
> > The main focus was to delete as much code as possible, I've counted
> > around 550 lines, which is not bad. Apart from that there are some
> > patches enforcing proper kernel APIs usage.
> > 
> > The only patch that really changes code is the
> > vchiq_ioc_copy_element_data() rewrite.
> > 
> > The last commit updates the TODO list with some of my observations, I
> > realise some of the might be a little opinionated. If anything it's
> > going to force a discussion on the topic, which is nice.
> > 
> > It was developed on top of the latest linux-next, and was tested on a
> > RPIv3B+ with audio, video and running vchiq_test.
> > 
> > Regards,
> > Nicolas
> > 
> 
> without a changelog i won't start a review.

What do you mean by this?  The individual patches have a "changelog" in
them, this is just a summary of the overall changes.

What do you feel is missing here?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 00/16] staging: vchiq: dead code removal & misc fixes

[Nicolas Saenz Julienne]

On Tue, 2018-11-20 at 10:57 +0100, Greg KH wrote:
> On Sun, Nov 18, 2018 at 04:55:49PM +0100, Stefan Wahren wrote:
> > Hi Nicolas,
> > 
> > > Nicolas Saenz Julienne  hat am 14.
> > > November 2018 um 13:59 geschrieben:
> > > 
> > > 
> > > Hi All,
> > > 
> > > This series was written in parallel with reading and
> > > understanding the
> > > vchiq code. So excuse me for the lack of logic in the sequence of
> > > patches.
> > > 
> > > The main focus was to delete as much code as possible, I've
> > > counted
> > > around 550 lines, which is not bad. Apart from that there are
> > > some
> > > patches enforcing proper kernel APIs usage.
> > > 
> > > The only patch that really changes code is the
> > > vchiq_ioc_copy_element_data() rewrite.
> > > 
> > > The last commit updates the TODO list with some of my
> > > observations, I
> > > realise some of the might be a little opinionated. If anything
> > > it's
> > > going to force a discussion on the topic, which is nice.
> > > 
> > > It was developed on top of the latest linux-next, and was tested
> > > on a
> > > RPIv3B+ with audio, video and running vchiq_test.
> > > 
> > > Regards,
> > > Nicolas
> > > 
> > 
> > without a changelog i won't start a review.
> 
> What do you mean by this?  The individual patches have a "changelog"
> in
> them, this is just a summary of the overall changes.
> 
> What do you feel is missing here?

Hi Greg,
I did send an RFC version of this, to which Stephan made several
comments. I should have added the changelog to the proper series
submission. I've been busy with work, but I'll send the updated version
during the day.

Regards,
Nicolas

> 
> thanks,
> 
> greg k-h



signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 00/16] staging: vchiq: dead code removal & misc fixes

[Stefan Wahren]

Am 20.11.18 um 11:04 schrieb Nicolas Saenz Julienne:
> On Tue, 2018-11-20 at 10:57 +0100, Greg KH wrote:
>> On Sun, Nov 18, 2018 at 04:55:49PM +0100, Stefan Wahren wrote:
>>> Hi Nicolas,
>>>
>>>
>>> without a changelog i won't start a review.
>> What do you mean by this?  The individual patches have a "changelog"
>> in
>> them, this is just a summary of the overall changes.
>>
>> What do you feel is missing here?
> Hi Greg,
> I did send an RFC version of this, to which Stephan made several
> comments. I should have added the changelog to the proper series
> submission. I've been busy with work, but I'll send the updated version
> during the day.
Thanks Stefan
>
> Regards,
> Nicolas
>
>> thanks,
>>
>> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] media: davinci_vpfe: bail out if kmalloc failed

[Nicholas Mc Guire]

 The kmalloc is passed indirectly to  from  but with an offset
which if not 0 will cause the null check if (to && from && size) 
to succeed. An explicit !NULL check is thus added for params here.

 ipipe_s_config and ipipe_g_config - both fail to check kmalloc
are called from ipipe_ioctl where a negative return is a valid
indication of error so simply setting rval = -ENOMEM seems ok.

Signed-off-by: Nicholas Mc Guire 
Fixes: da43b6ccadcf ("[media] davinci: vpfe: dm365: add IPIPE support for media 
controller driver")
---

Problem located with experimental coccinelle patch

Patch was compile tested with: davinci_all_defconfig + SAGING=y,
STAGING_MEDIA=y, MEDIA_SUPPORT=m, MEDIA_CONTROLLER=y,
VIDEO_V4L2_SUBDEV_API=y, VIDEO_DAVINCI_VPBE_DISPLAY=m,
VIDEO_DM365_VPFE=m
(with some coccicheck findings unrelated to the proposed change)

Patch is against 4.20-rc3 (localversion-next is next-20181120)

 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 3d910b8..0150aed 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1266,6 +1266,11 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct 
vpfe_ipipe_config *cfg)
 
params = kmalloc(sizeof(struct ipipe_module_params),
 GFP_KERNEL);
+   if (!params) {
+   rval = -ENOMEM;
+   goto error;
+   }
+
to = (void *)params + module_if->param_offset;
size = module_if->param_size;
 
@@ -1308,6 +1313,11 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct 
vpfe_ipipe_config *cfg)
 
params = kmalloc(sizeof(struct ipipe_module_params),
 GFP_KERNEL);
+   if (!params) {
+   rval = -ENOMEM;
+   goto error;
+   }
+
from = (void *)params + module_if->param_offset;
size = module_if->param_size;
 
-- 
2.1.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH net-next 3/4] benet: use skb_vlan_tag_get_prio()

[Michał Mirosław]

Signed-off-by: Michał Mirosław 
---
 drivers/net/ethernet/emulex/benet/be_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c 
b/drivers/net/ethernet/emulex/benet/be_main.c
index 80b2bd3747ce..245abf0d19c0 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -796,7 +796,7 @@ static inline u16 be_get_tx_vlan_tag(struct be_adapter 
*adapter,
u16 vlan_tag;
 
vlan_tag = skb_vlan_tag_get(skb);
-   vlan_prio = (vlan_tag & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+   vlan_prio = skb_vlan_tag_get_prio(skb);
/* If vlan priority provided by OS is NOT in available bmap */
if (!(adapter->vlan_prio_bmap & (1 << vlan_prio)))
vlan_tag = (vlan_tag & ~VLAN_PRIO_MASK) |
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH net-next 4/4] mlx5: use skb_vlan_tag_get_prio()

[Michał Mirosław]

Signed-off-by: Michał Mirosław 
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index 6dacaeba2fbf..9afdf955f2bc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -127,7 +127,7 @@ u16 mlx5e_select_queue(struct net_device *dev, struct 
sk_buff *skb,
else
 #endif
if (skb_vlan_tag_present(skb))
-   up = skb->vlan_tci >> VLAN_PRIO_SHIFT;
+   up = skb_vlan_tag_get_prio(skb);
 
/* channel_ix can be larger than num_channels since
 * dev->num_real_tx_queues = num_channels * num_tc
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH net-next 2/4] net/hyperv: use skb_vlan_tag_*() helpers

[Michał Mirosław]

Replace open-coded bitfield manipulation with skb_vlan_tag_*() helpers.
This also enables correctly passing of VLAN.CFI bit.

Signed-off-by: Michał Mirosław 
---
 drivers/net/hyperv/netvsc_drv.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index cf36e7ff3191..85936ed9e952 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -605,9 +605,9 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct 
net_device *net)
 IEEE_8021Q_INFO);
 
vlan->value = 0;
-   vlan->vlanid = skb->vlan_tci & VLAN_VID_MASK;
-   vlan->pri = (skb->vlan_tci & VLAN_PRIO_MASK) >>
-   VLAN_PRIO_SHIFT;
+   vlan->vlanid = skb_vlan_tag_get_id(skb);
+   vlan->cfi = skb_vlan_tag_get_cfi(skb);
+   vlan->pri = skb_vlan_tag_get_prio(skb);
}
 
if (skb_is_gso(skb)) {
@@ -781,7 +781,8 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct 
net_device *net,
}
 
if (vlan) {
-   u16 vlan_tci = vlan->vlanid | (vlan->pri << VLAN_PRIO_SHIFT);
+   u16 vlan_tci = vlan->vlanid | (vlan->pri << VLAN_PRIO_SHIFT) |
+   (vlan->cfi ? VLAN_CFI_MASK : 0);
 
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
   vlan_tci);
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH net-next 1/4] net/vlan: introduce skb_vlan_tag_get_cfi() helper

[Michał Mirosław]

Abstract CFI/DEI bit access consistently with other VLAN tag fields.

Signed-off-by: Michał Mirosław 
---
 include/linux/if_vlan.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 7a541eadf78e..4cca4da7a6de 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -65,7 +65,7 @@ static inline struct vlan_ethhdr *vlan_eth_hdr(const struct 
sk_buff *skb)
 
 #define VLAN_PRIO_MASK 0xe000 /* Priority Code Point */
 #define VLAN_PRIO_SHIFT13
-#define VLAN_CFI_MASK  0x1000 /* Canonical Format Indicator */
+#define VLAN_CFI_MASK  0x1000 /* Canonical Format Indicator / Drop 
Eligible Indicator */
 #define VLAN_VID_MASK  0x0fff /* VLAN Identifier */
 #define VLAN_N_VID 4096
 
@@ -80,6 +80,7 @@ static inline bool is_vlan_dev(const struct net_device *dev)
 #define skb_vlan_tag_present(__skb)((__skb)->vlan_present)
 #define skb_vlan_tag_get(__skb)((__skb)->vlan_tci)
 #define skb_vlan_tag_get_id(__skb) ((__skb)->vlan_tci & VLAN_VID_MASK)
+#define skb_vlan_tag_get_cfi(__skb)(!!((__skb)->vlan_tci & VLAN_CFI_MASK))
 #define skb_vlan_tag_get_prio(__skb)   (((__skb)->vlan_tci & VLAN_PRIO_MASK) 
>> VLAN_PRIO_SHIFT)
 
 static inline int vlan_get_rx_ctag_filter_info(struct net_device *dev)
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH net-next 0/4] VLAN tag handling cleanup

[Michał Mirosław]

This is a cleanup set after VLAN_TAG_PRESENT removal. The CFI bit
handling is made similar to how other tag fields are used.

Michał Mirosław (4):
  net/vlan: introduce skb_vlan_tag_get_cfi() helper
  net/hyperv: use skb_vlan_tag_*() helpers
  benet: use skb_vlan_tag_get_prio()
  mlx5: use skb_vlan_tag_get_prio()

 drivers/net/ethernet/emulex/benet/be_main.c | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 2 +-
 drivers/net/hyperv/netvsc_drv.c | 9 +
 include/linux/if_vlan.h | 3 ++-
 4 files changed, 9 insertions(+), 7 deletions(-)

-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/10] staging: erofs: fix race when the managed cache is enabled

[Gao Xiang]

When the managed cache is enabled, the last reference count
of a workgroup must be used for its workstation.

Otherwise, it could lead to incorrect (un)freezes in
the reclaim path, and it would be harmful.

A typical race as follows:

Thread 1 (In the reclaim path)  Thread 2
workgroup_freeze(grp, 1)refcnt = 1
...
workgroup_unfreeze(grp, 1)  refcnt = 1
workgroup_get(grp)  refcnt = 2 (x)
workgroup_put(grp)  refcnt = 1 (x)
...unexpected behaviors

* grp is detached but still used, which violates cache-managed
  freeze constraint.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/internal.h |   1 +
 drivers/staging/erofs/utils.c| 131 +++
 2 files changed, 93 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 57575c7f5635..89dbd0888e53 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -250,6 +250,7 @@ static inline bool erofs_workgroup_get(struct 
erofs_workgroup *grp, int *ocnt)
 }
 
 #define __erofs_workgroup_get(grp) atomic_inc(&(grp)->refcount)
+#define __erofs_workgroup_put(grp) atomic_dec(&(grp)->refcount)
 
 extern int erofs_workgroup_put(struct erofs_workgroup *grp);
 
diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index ea8a962e5c95..90de8d9195b7 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -83,12 +83,21 @@ int erofs_register_workgroup(struct super_block *sb,
 
grp = xa_tag_pointer(grp, tag);
 
-   err = radix_tree_insert(&sbi->workstn_tree,
-   grp->index, grp);
+   /*
+* Bump up reference count before making this workgroup
+* visible to other users in order to avoid potential UAF
+* without serialized by erofs_workstn_lock.
+*/
+   __erofs_workgroup_get(grp);
 
-   if (!err) {
-   __erofs_workgroup_get(grp);
-   }
+   err = radix_tree_insert(&sbi->workstn_tree,
+   grp->index, grp);
+   if (unlikely(err))
+   /*
+* it's safe to decrease since the workgroup isn't visible
+* and refcount >= 2 (cannot be freezed).
+*/
+   __erofs_workgroup_put(grp);
 
erofs_workstn_unlock(sbi);
radix_tree_preload_end();
@@ -97,19 +106,91 @@ int erofs_register_workgroup(struct super_block *sb,
 
 extern void erofs_workgroup_free_rcu(struct erofs_workgroup *grp);
 
+static void  __erofs_workgroup_free(struct erofs_workgroup *grp)
+{
+   atomic_long_dec(&erofs_global_shrink_cnt);
+   erofs_workgroup_free_rcu(grp);
+}
+
 int erofs_workgroup_put(struct erofs_workgroup *grp)
 {
int count = atomic_dec_return(&grp->refcount);
 
if (count == 1)
atomic_long_inc(&erofs_global_shrink_cnt);
-   else if (!count) {
-   atomic_long_dec(&erofs_global_shrink_cnt);
-   erofs_workgroup_free_rcu(grp);
-   }
+   else if (!count)
+   __erofs_workgroup_free(grp);
return count;
 }
 
+#ifdef EROFS_FS_HAS_MANAGED_CACHE
+/* for cache-managed case, customized reclaim paths exist */
+static void erofs_workgroup_unfreeze_final(struct erofs_workgroup *grp)
+{
+   erofs_workgroup_unfreeze(grp, 0);
+   __erofs_workgroup_free(grp);
+}
+
+bool erofs_try_to_release_workgroup(struct erofs_sb_info *sbi,
+   struct erofs_workgroup *grp,
+   bool cleanup)
+{
+   /*
+* for managed cache enabled, the refcount of workgroups
+* themselves could be < 0 (freezed). So there is no guarantee
+* that all refcount > 0 if managed cache is enabled.
+*/
+   if (!erofs_workgroup_try_to_freeze(grp, 1))
+   return false;
+
+   /*
+* note that all cached pages should be unlinked
+* before delete it from the radix tree.
+* Otherwise some cached pages of an orphan old workgroup
+* could be still linked after the new one is available.
+*/
+   if (erofs_try_to_free_all_cached_pages(sbi, grp)) {
+   erofs_workgroup_unfreeze(grp, 1);
+   return false;
+   }
+
+   /* it is impossible to fail after we freeze the workgroup */
+   BUG_ON(xa_untag_pointer(radix_tree_delete(&sbi->workstn_tree,
+ grp->index)) != grp);
+
+   /*
+* if managed cache is enable, the last refcount
+* should indicate the related workstation.
+*/
+   erofs_workgroup_unfreeze_final(grp);
+   return true;
+}
+
+#else
+/* for nocache case, no customized reclaim path at all */
+bool erofs_try_to_release_workgroup(struct erofs_sb_info

[PATCH 00/10] staging: erofs: decompression stability enhancement

[Gao Xiang]

Hi,

This patchset mainly focuses on erofs decompression stability, most of
them are found in the internal beta test. Some patches which are still
in testing or reviewing aren't included in this patchset and will be
sent after carefully testing.

Thanks,
Gao Xiang

Gao Xiang (10):
  staging: erofs: fix `trace_erofs_readpage' position
  staging: erofs: fix race when the managed cache is enabled
  staging: erofs: atomic_cond_read_relaxed on ref-locked workgroup
  staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'
  staging: erofs: add a full barrier in erofs_workgroup_unfreeze
  staging: erofs: fix the definition of DBG_BUGON
  staging: erofs: separate into init_once / always
  staging: erofs: locked before registering for all new workgroups
  staging: erofs: decompress asynchronously if PG_readahead page at
first
  staging: erofs: rename strange variable names in z_erofs_vle_frontend

 drivers/staging/erofs/internal.h  |  69 
 drivers/staging/erofs/unzip_vle.c |  78 ---
 drivers/staging/erofs/utils.c | 131 ++
 3 files changed, 190 insertions(+), 88 deletions(-)

-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/10] staging: erofs: fix `trace_erofs_readpage' position

[Gao Xiang]

`trace_erofs_readpage' should be placed in .readpage()
rather than in the internal `z_erofs_do_read_page'.

Fixes: 284db12cfda3 ("staging: erofs: add trace points for reading zipped data")

Reviewed-by: Chen Gong 
Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/unzip_vle.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c 
b/drivers/staging/erofs/unzip_vle.c
index 6a283f618f46..ede3383ac601 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -598,8 +598,6 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend 
*fe,
unsigned int cur, end, spiltted, index;
int err = 0;
 
-   trace_erofs_readpage(page, false);
-
/* register locked file pages as online pages in pack */
z_erofs_onlinepage_init(page);
 
@@ -1288,6 +1286,8 @@ static int z_erofs_vle_normalaccess_readpage(struct file 
*file,
int err;
LIST_HEAD(pagepool);
 
+   trace_erofs_readpage(page, false);
+
 #if (EROFS_FS_ZIP_CACHE_LVL >= 2)
f.cachedzone_la = (erofs_off_t)page->index << PAGE_SHIFT;
 #endif
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/10] staging: erofs: atomic_cond_read_relaxed on ref-locked workgroup

[Gao Xiang]

It's better to use atomic_cond_read_relaxed, which is implemented
in hardware instructions to monitor a variable changes currently
for ARM64, instead of open-coded busy waiting.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/internal.h | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 89dbd0888e53..eb80ba44d072 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -221,23 +221,29 @@ static inline void erofs_workgroup_unfreeze(
preempt_enable();
 }
 
+#if defined(CONFIG_SMP)
+static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
+{
+   return atomic_cond_read_relaxed(&grp->refcount,
+   VAL != EROFS_LOCKED_MAGIC);
+}
+#else
+static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
+{
+   int v = atomic_read(&grp->refcount);
+
+   /* workgroup is never freezed on uniprocessor systems */
+   DBG_BUGON(v == EROFS_LOCKED_MAGIC);
+   return v;
+}
+#endif
+
 static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 {
-   const int locked = (int)EROFS_LOCKED_MAGIC;
int o;
 
 repeat:
-   o = atomic_read(&grp->refcount);
-
-   /* spin if it is temporarily locked at the reclaim path */
-   if (unlikely(o == locked)) {
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-   do
-   cpu_relax();
-   while (atomic_read(&grp->refcount) == locked);
-#endif
-   goto repeat;
-   }
+   o = erofs_wait_on_workgroup_freezed(grp);
 
if (unlikely(o <= 0))
return -1;
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/10] staging: erofs: add a full barrier in erofs_workgroup_unfreeze

[Gao Xiang]

Just like other generic locks, insert a full barrier
in case of memory reorder.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/internal.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 2e0ef92c138b..f77653d33633 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -209,6 +209,7 @@ static inline bool erofs_workgroup_try_to_freeze(struct 
erofs_workgroup *grp,
 static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp,
int orig_val)
 {
+   smp_mb();
atomic_set(&grp->refcount, orig_val);
preempt_enable();
 }
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/10] staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'

[Gao Xiang]

There are two minor issues in the current freeze interface:

   1) Freeze interfaces have not related with CONFIG_DEBUG_SPINLOCK,
  therefore fix the incorrect conditions;

   2) For SMP platforms, it should also disable preemption before
  doing atomic_cmpxchg in case that some high priority tasks
  preempt between atomic_cmpxchg and disable_preempt, then spin
  on the locked refcount later.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/internal.h | 41 
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index eb80ba44d072..2e0ef92c138b 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -194,40 +194,49 @@ struct erofs_workgroup {
 
 #define EROFS_LOCKED_MAGIC (INT_MIN | 0xE0F510CCL)
 
-static inline bool erofs_workgroup_try_to_freeze(
-   struct erofs_workgroup *grp, int v)
+#if defined(CONFIG_SMP)
+static inline bool erofs_workgroup_try_to_freeze(struct erofs_workgroup *grp,
+int val)
 {
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-   if (v != atomic_cmpxchg(&grp->refcount,
-   v, EROFS_LOCKED_MAGIC))
-   return false;
preempt_disable();
-#else
-   preempt_disable();
-   if (atomic_read(&grp->refcount) != v) {
+   if (val != atomic_cmpxchg(&grp->refcount, val, EROFS_LOCKED_MAGIC)) {
preempt_enable();
return false;
}
-#endif
return true;
 }
 
-static inline void erofs_workgroup_unfreeze(
-   struct erofs_workgroup *grp, int v)
+static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp,
+   int orig_val)
 {
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-   atomic_set(&grp->refcount, v);
-#endif
+   atomic_set(&grp->refcount, orig_val);
preempt_enable();
 }
 
-#if defined(CONFIG_SMP)
 static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
 {
return atomic_cond_read_relaxed(&grp->refcount,
VAL != EROFS_LOCKED_MAGIC);
 }
 #else
+static inline bool erofs_workgroup_try_to_freeze(struct erofs_workgroup *grp,
+int val)
+{
+   preempt_disable();
+   /* no need to spin on UP platforms, let's just disable preemption. */
+   if (val != atomic_read(&grp->refcount)) {
+   preempt_enable();
+   return false;
+   }
+   return true;
+}
+
+static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp,
+   int orig_val)
+{
+   preempt_enable();
+}
+
 static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
 {
int v = atomic_read(&grp->refcount);
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/10] staging: erofs: fix the definition of DBG_BUGON

[Gao Xiang]

It's better not to positively BUG_ON the kernel, however developers
need a way to locate issues as soon as possible.

DBG_BUGON is introduced and it could only crash when EROFS_FS_DEBUG
(EROFS developping feature) is on. It is helpful for developers
to find and solve bugs quickly.

Previously, DBG_BUGON is defined as ((void)0) if EROFS_FS_DEBUG is off,
but some unused variable warnings as follows could occur:

drivers/staging/erofs/unzip_vle.c: In function ‘init_always’:
drivers/staging/erofs/unzip_vle.c:61:33: warning: unused variable ‘work’ 
[-Wunused-variable]
  struct z_erofs_vle_work *const work =
 ^~~~

Fix it to #define DBG_BUGON(x) ((void)(x)).

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index f77653d33633..0aa2a41b9885 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -39,7 +39,7 @@
 #define debugln(x, ...) ((void)0)
 
 #define dbg_might_sleep()   ((void)0)
-#define DBG_BUGON(...)  ((void)0)
+#define DBG_BUGON(x)((void)(x))
 #endif
 
 enum {
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/10] staging: erofs: separate into init_once / always

[Gao Xiang]

`z_erofs_vle_workgroup' is heavily generated in the decompression,
for example, it resets 32 bytes redundantly for 64-bit platforms
even through Z_EROFS_VLE_INLINE_PAGEVECS + Z_EROFS_CLUSTER_MAX_PAGES,
default 4, pages are stored in `z_erofs_vle_workgroup'.

As an another example, `struct mutex' takes 72 bytes for our kirin
64-bit platforms, it's unnecessary to be reseted at first and
be initialized each time.

Let's avoid filling all `z_erofs_vle_workgroup' with 0 at first
since most fields are reinitialized to meaningful values later,
and pagevec is no need to initialized at all.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/unzip_vle.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c 
b/drivers/staging/erofs/unzip_vle.c
index ede3383ac601..4e5843e8ee35 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -43,12 +43,38 @@ static inline int init_unzip_workqueue(void)
return z_erofs_workqueue ? 0 : -ENOMEM;
 }
 
+static void init_once(void *ptr)
+{
+   struct z_erofs_vle_workgroup *grp = ptr;
+   struct z_erofs_vle_work *const work =
+   z_erofs_vle_grab_primary_work(grp);
+   unsigned int i;
+
+   mutex_init(&work->lock);
+   work->nr_pages = 0;
+   work->vcnt = 0;
+   for (i = 0; i < Z_EROFS_CLUSTER_MAX_PAGES; ++i)
+   grp->compressed_pages[i] = NULL;
+}
+
+static void init_always(struct z_erofs_vle_workgroup *grp)
+{
+   struct z_erofs_vle_work *const work =
+   z_erofs_vle_grab_primary_work(grp);
+
+   atomic_set(&grp->obj.refcount, 1);
+   grp->flags = 0;
+
+   DBG_BUGON(work->nr_pages);
+   DBG_BUGON(work->vcnt);
+}
+
 int __init z_erofs_init_zip_subsystem(void)
 {
z_erofs_workgroup_cachep =
kmem_cache_create("erofs_compress",
  Z_EROFS_WORKGROUP_SIZE, 0,
- SLAB_RECLAIM_ACCOUNT, NULL);
+ SLAB_RECLAIM_ACCOUNT, init_once);
 
if (z_erofs_workgroup_cachep) {
if (!init_unzip_workqueue())
@@ -370,10 +396,11 @@ z_erofs_vle_work_register(const struct 
z_erofs_vle_work_finder *f,
BUG_ON(grp);
 
/* no available workgroup, let's allocate one */
-   grp = kmem_cache_zalloc(z_erofs_workgroup_cachep, GFP_NOFS);
+   grp = kmem_cache_alloc(z_erofs_workgroup_cachep, GFP_NOFS);
if (unlikely(!grp))
return ERR_PTR(-ENOMEM);
 
+   init_always(grp);
grp->obj.index = f->idx;
grp->llen = map->m_llen;
 
@@ -381,7 +408,6 @@ z_erofs_vle_work_register(const struct 
z_erofs_vle_work_finder *f,
(map->m_flags & EROFS_MAP_ZIPPED) ?
Z_EROFS_VLE_WORKGRP_FMT_LZ4 :
Z_EROFS_VLE_WORKGRP_FMT_PLAIN);
-   atomic_set(&grp->obj.refcount, 1);
 
/* new workgrps have been claimed as type 1 */
WRITE_ONCE(grp->next, *f->owned_head);
@@ -394,8 +420,6 @@ z_erofs_vle_work_register(const struct 
z_erofs_vle_work_finder *f,
work = z_erofs_vle_grab_primary_work(grp);
work->pageofs = f->pageofs;
 
-   mutex_init(&work->lock);
-
if (gnew) {
int err = erofs_register_workgroup(f->sb, &grp->obj, 0);
 
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 10/10] staging: erofs: rename strange variable names in z_erofs_vle_frontend

[Gao Xiang]

Previously, 2 members called `initial' and `cachedzone_la' are used
for applying caching policy (whether the workgroup is at either end),
which are hard to understand, rename them to `backmost' and `headoffset'.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/unzip_vle.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c 
b/drivers/staging/erofs/unzip_vle.c
index 824d2c12c2f3..1ef178e7ac39 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -586,10 +586,9 @@ struct z_erofs_vle_frontend {
 
z_erofs_vle_owned_workgrp_t owned_head;
 
-   bool initial;
-#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
-   erofs_off_t cachedzone_la;
-#endif
+   /* used for applying cache strategy on the fly */
+   bool backmost;
+   erofs_off_t headoffset;
 };
 
 #define VLE_FRONTEND_INIT(__i) { \
@@ -600,7 +599,7 @@ struct z_erofs_vle_frontend {
}, \
.builder = VLE_WORK_BUILDER_INIT(), \
.owned_head = Z_EROFS_VLE_WORKGRP_TAIL, \
-   .initial = true, }
+   .backmost = true, }
 
 static int z_erofs_do_read_page(struct z_erofs_vle_frontend *fe,
struct page *page,
@@ -643,7 +642,7 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend 
*fe,
debugln("%s: [out-of-range] pos %llu", __func__, offset + cur);
 
if (z_erofs_vle_work_iter_end(builder))
-   fe->initial = false;
+   fe->backmost = false;
 
map->m_la = offset + cur;
map->m_llen = 0;
@@ -669,8 +668,8 @@ static int z_erofs_do_read_page(struct z_erofs_vle_frontend 
*fe,
erofs_blknr(map->m_pa),
grp->compressed_pages, erofs_blknr(map->m_plen),
/* compressed page caching selection strategy */
-   fe->initial | (EROFS_FS_ZIP_CACHE_LVL >= 2 ?
-   map->m_la < fe->cachedzone_la : 0));
+   fe->backmost | (EROFS_FS_ZIP_CACHE_LVL >= 2 ?
+   map->m_la < fe->headoffset : 0));
 
if (noio_outoforder && builder_is_followed(builder))
builder->role = Z_EROFS_VLE_WORK_PRIMARY;
@@ -1316,9 +1315,8 @@ static int z_erofs_vle_normalaccess_readpage(struct file 
*file,
 
trace_erofs_readpage(page, false);
 
-#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
-   f.cachedzone_la = (erofs_off_t)page->index << PAGE_SHIFT;
-#endif
+   f.headoffset = (erofs_off_t)page->index << PAGE_SHIFT;
+
err = z_erofs_do_read_page(&f, page, &pagepool);
(void)z_erofs_vle_work_iter_end(&f.builder);
 
@@ -1354,9 +1352,8 @@ static int z_erofs_vle_normalaccess_readpages(struct file 
*filp,
trace_erofs_readpages(mapping->host, lru_to_page(pages),
  nr_pages, false);
 
-#if (EROFS_FS_ZIP_CACHE_LVL >= 2)
-   f.cachedzone_la = (erofs_off_t)lru_to_page(pages)->index << PAGE_SHIFT;
-#endif
+   f.headoffset = (erofs_off_t)lru_to_page(pages)->index << PAGE_SHIFT;
+
for (; nr_pages; --nr_pages) {
struct page *page = lru_to_page(pages);
 
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 08/10] staging: erofs: locked before registering for all new workgroups

[Gao Xiang]

Let's make sure that the one registering a workgroup will also
take the primary work lock at first for two reasons:
  1) There's no need to introduce such a race window (and consequently
 overhead) between registering and locking, other tasks could break
 in by chance, and the race seems unnecessary (no benefit at all);

  2) It's better to take the primary work when a workgroup
 is registered to apply the cache managed policy, for example,
 if some other tasks break in, it could turn into the in-place
 decompression rather than use as the cached decompression.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/unzip_vle.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c 
b/drivers/staging/erofs/unzip_vle.c
index 4e5843e8ee35..a1376f3c6065 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -420,18 +420,22 @@ z_erofs_vle_work_register(const struct 
z_erofs_vle_work_finder *f,
work = z_erofs_vle_grab_primary_work(grp);
work->pageofs = f->pageofs;
 
+   /* lock all primary followed works before visible to others */
+   if (unlikely(!mutex_trylock(&work->lock)))
+   /* for a new workgroup, try_lock *never* fails */
+   DBG_BUGON(1);
+
if (gnew) {
int err = erofs_register_workgroup(f->sb, &grp->obj, 0);
 
if (err) {
+   mutex_unlock(&work->lock);
kmem_cache_free(z_erofs_workgroup_cachep, grp);
return ERR_PTR(-EAGAIN);
}
}
 
*f->owned_head = *f->grp_ret = grp;
-
-   mutex_lock(&work->lock);
return work;
 }
 
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/10] staging: erofs: decompress asynchronously if PG_readahead page at first

[Gao Xiang]

For the case of nr_to_read == lookahead_size, it is better to
decompress asynchronously as well since no page will be needed immediately.

Reviewed-by: Chao Yu 
Signed-off-by: Gao Xiang 
---
 drivers/staging/erofs/unzip_vle.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/unzip_vle.c 
b/drivers/staging/erofs/unzip_vle.c
index a1376f3c6065..824d2c12c2f3 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1344,8 +1344,8 @@ static int z_erofs_vle_normalaccess_readpages(struct file 
*filp,
 {
struct inode *const inode = mapping->host;
struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
-   const bool sync = __should_decompress_synchronously(sbi, nr_pages);
 
+   bool sync = __should_decompress_synchronously(sbi, nr_pages);
struct z_erofs_vle_frontend f = VLE_FRONTEND_INIT(inode);
gfp_t gfp = mapping_gfp_constraint(mapping, GFP_KERNEL);
struct page *head = NULL;
@@ -1363,6 +1363,13 @@ static int z_erofs_vle_normalaccess_readpages(struct 
file *filp,
prefetchw(&page->flags);
list_del(&page->lru);
 
+   /*
+* A pure asynchronous readahead is indicated if
+* a PG_readahead marked page is hitted at first.
+* Let's also do asynchronous decompression for this case.
+*/
+   sync &= !(PageReadahead(page) && !head);
+
if (add_to_page_cache_lru(page, mapping, page->index, gfp)) {
list_add(&page->lru, &pagepool);
continue;
-- 
2.14.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/16] staging: vchiq_core: rework vchiq_get_config

[Nicolas Saenz Julienne]

The function is overly complicated for what it's ultimately achieving.
It's simply filling up a structure.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_arm.c   | 12 
 .../interface/vchiq_arm/vchiq_core.c  | 30 +--
 .../interface/vchiq_arm/vchiq_if.h|  3 +-
 3 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 45de21c210c1..5af3f2651bd3 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1480,13 +1480,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
unsigned long arg)
ret = -EINVAL;
break;
}
-   status = vchiq_get_config(instance, args.config_size, &config);
-   if (status == VCHIQ_SUCCESS) {
-   if (copy_to_user((void __user *)args.pconfig,
-   &config, args.config_size) != 0) {
-   ret = -EFAULT;
-   break;
-   }
+
+   vchiq_get_config(&config);
+   if (copy_to_user(args.pconfig, &config, args.config_size)) {
+   ret = -EFAULT;
+   break;
}
} break;
 
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 7642ced31436..89f1ccdc3b98 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3583,28 +3583,14 @@ vchiq_get_peer_version(VCHIQ_SERVICE_HANDLE_T handle, 
short *peer_version)
return status;
 }
 
-VCHIQ_STATUS_T
-vchiq_get_config(VCHIQ_INSTANCE_T instance,
-   int config_size, VCHIQ_CONFIG_T *pconfig)
-{
-   VCHIQ_CONFIG_T config;
-
-   (void)instance;
-
-   config.max_msg_size   = VCHIQ_MAX_MSG_SIZE;
-   config.bulk_threshold = VCHIQ_MAX_MSG_SIZE;
-   config.max_outstanding_bulks  = VCHIQ_NUM_SERVICE_BULKS;
-   config.max_services   = VCHIQ_MAX_SERVICES;
-   config.version= VCHIQ_VERSION;
-   config.version_min= VCHIQ_VERSION_MIN;
-
-   if (config_size > sizeof(VCHIQ_CONFIG_T))
-   return VCHIQ_ERROR;
-
-   memcpy(pconfig, &config,
-   min(config_size, (int)(sizeof(VCHIQ_CONFIG_T;
-
-   return VCHIQ_SUCCESS;
+void vchiq_get_config(VCHIQ_CONFIG_T *config)
+{
+   config->max_msg_size   = VCHIQ_MAX_MSG_SIZE;
+   config->bulk_threshold = VCHIQ_MAX_MSG_SIZE;
+   config->max_outstanding_bulks  = VCHIQ_NUM_SERVICE_BULKS;
+   config->max_services   = VCHIQ_MAX_SERVICES;
+   config->version= VCHIQ_VERSION;
+   config->version_min= VCHIQ_VERSION_MIN;
 }
 
 VCHIQ_STATUS_T
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
index e4109a83e628..87829a244465 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
@@ -164,8 +164,7 @@ extern VCHIQ_STATUS_T 
vchiq_bulk_receive_handle(VCHIQ_SERVICE_HANDLE_T service,
 extern int   vchiq_get_client_id(VCHIQ_SERVICE_HANDLE_T service);
 extern void *vchiq_get_service_userdata(VCHIQ_SERVICE_HANDLE_T service);
 extern int   vchiq_get_service_fourcc(VCHIQ_SERVICE_HANDLE_T service);
-extern VCHIQ_STATUS_T vchiq_get_config(VCHIQ_INSTANCE_T instance,
-   int config_size, VCHIQ_CONFIG_T *pconfig);
+extern void vchiq_get_config(VCHIQ_CONFIG_T *config);
 extern VCHIQ_STATUS_T vchiq_set_service_option(VCHIQ_SERVICE_HANDLE_T service,
VCHIQ_SERVICE_OPTION_T option, int value);
 
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/16] staging: vchiq_arm: get rid of vchi_mh.h

[Nicolas Saenz Julienne]

The concept of VCHI_MEM_HANDLE_T is introduced by this header file and
was meant to be used with bulk transfers. After a quick look in
vchiq_core.c it is pretty clear that it actually accomplishes nothing
nor alters the bulk transfers in any way.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../vc04_services/interface/vchi/vchi.h   |  3 --
 .../vc04_services/interface/vchi/vchi_mh.h| 42 ---
 .../interface/vchiq_arm/vchiq_2835_arm.c  |  6 +--
 .../interface/vchiq_arm/vchiq_arm.c   | 27 ++--
 .../interface/vchiq_arm/vchiq_core.c  | 17 
 .../interface/vchiq_arm/vchiq_core.h  | 10 ++---
 .../interface/vchiq_arm/vchiq_if.h|  8 ++--
 7 files changed, 27 insertions(+), 86 deletions(-)
 delete mode 100644 drivers/staging/vc04_services/interface/vchi/vchi_mh.h

diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h 
b/drivers/staging/vc04_services/interface/vchi/vchi.h
index 379a16ebfd5b..e326926eac31 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi.h
@@ -36,7 +36,6 @@
 
 #include "interface/vchi/vchi_cfg.h"
 #include "interface/vchi/vchi_common.h"
-#include "vchi_mh.h"
 
 /**
  Global defs
@@ -239,7 +238,6 @@ extern int32_t 
vchi_bulk_queue_receive(VCHI_SERVICE_HANDLE_T handle,
 
 // Prepare interface for a transfer from the other side into relocatable 
memory.
 int32_t vchi_bulk_queue_receive_reloc(const VCHI_SERVICE_HANDLE_T handle,
- VCHI_MEM_HANDLE_T h_dst,
  uint32_t offset,
  uint32_t data_size,
  const VCHI_FLAGS_T flags,
@@ -261,7 +259,6 @@ extern int32_t 
vchi_bulk_queue_transmit(VCHI_SERVICE_HANDLE_T handle,
 #endif
 
 extern int32_t vchi_bulk_queue_transmit_reloc(VCHI_SERVICE_HANDLE_T handle,
- VCHI_MEM_HANDLE_T h_src,
  uint32_t offset,
  uint32_t data_size,
  VCHI_FLAGS_T flags,
diff --git a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h 
b/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
deleted file mode 100644
index 198bd076b666..
--- a/drivers/staging/vc04_services/interface/vchi/vchi_mh.h
+++ /dev/null
@@ -1,42 +0,0 @@
-/**
- * Copyright (c) 2010-2012 Broadcom. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions, and the following disclaimer,
- *without modification.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. The names of the above-listed copyright holders may not be used
- *to endorse or promote products derived from this software without
- *specific prior written permission.
- *
- * ALTERNATIVELY, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2, as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
- * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
- * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
- * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef VCHI_MH_H_
-#define VCHI_MH_H_
-
-#include 
-
-typedef int32_t VCHI_MEM_HANDLE_T;
-#define VCHI_MEM_HANDLE_INVALID 0
-
-#endif
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 83d740feab96..014583cdf367 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -247,13 +247,10 @@ remote_event_signal(REMOTE_EVENT_T *event)
 }
 
 VCHIQ_STATUS_T
-vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, VCHI_MEM_HANDLE_T memhandle,
-   void *offset, int siz

[PATCH 00/16] staging: vchiq: dead code removal & misc fixes

[Nicolas Saenz Julienne]

Hi All,

This series was written in parallel with reading and understanding the
vchiq code. So excuse me for the lack of logic in the sequence of
patches.

The main focus was to delete as much code as possible, I've counted
around 550 lines, which is not bad. Apart from that there are some
patches enforcing proper kernel APIs usage.

The only patch that really changes code is the
vchiq_ioc_copy_element_data() rewrite.

The last commit updates the TODO list with some of my observations, I
realise some of the might be a little opinionated. If anything it's
going to force a discussion on the topic, which is nice.

It was developed on top of the latest linux-next, and was tested on a
RPIv3B+ with audio, video and running vchiq_test.

Regards,
Nicolas

RFC -> PATCH, as per Stefan's comments:
  - Remove semaphore initialization from remove_event_create()
(commit 9)
  - Join all three semaphore to completion patches (commit 11)
  - Update probe/init commit message (commit 14)
  - Update TODO commit message and clean up (commit 16)
  - Fix spelling on some of the patches

===

Nicolas Saenz Julienne (16):
  staging: vchiq_core: rework vchiq_get_config
  staging: vchiq_arm: rework close/remove_service IOCTLS
  staging: vchiq_shim: delete vchi_service_create
  staging: vchiq_arm: use list_for_each_entry when accessing
bulk_waiter_list
  staging: vchiq_arm: get rid of vchi_mh.h
  staging: vchiq_arm: rework vchiq_ioc_copy_element_data
  staging: vchiq-core: get rid of is_master distinction
  staging: vchiq_core: remove unnecessary safety checks in
vchiq_init_state
  staging: vchiq_core: do not initialize semaphores twice
  staging: vchiq_core: don't add a wmb() before remote_event_signal()
  staging: vchiq: use completions instead of semaphores
  staging: vchiq_util: get rid of unneeded memory barriers
  staging: vchiq_core: fix logic redundancy in parse_open
  staging: vchiq_arm: rework probe and init functions
  staging: vchiq_arm: fix open/release cdev functions
  staging: vchiq: add more tasks to the TODO list

 .../staging/vc04_services/interface/vchi/TODO |  42 ++
 .../vc04_services/interface/vchi/vchi.h   |   8 -
 .../vc04_services/interface/vchi/vchi_mh.h|  42 --
 .../interface/vchiq_arm/vchiq_2835_arm.c  |  18 +-
 .../interface/vchiq_arm/vchiq_arm.c   | 598 --
 .../interface/vchiq_arm/vchiq_core.c  | 523 ---
 .../interface/vchiq_arm/vchiq_core.h  |  47 +-
 .../interface/vchiq_arm/vchiq_if.h|  11 +-
 .../interface/vchiq_arm/vchiq_shim.c  |  32 -
 .../interface/vchiq_arm/vchiq_util.c  |  48 +-
 .../interface/vchiq_arm/vchiq_util.h  |   6 +-
 11 files changed, 435 insertions(+), 940 deletions(-)
 delete mode 100644 drivers/staging/vc04_services/interface/vchi/vchi_mh.h

-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/16] staging: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list

[Nicolas Saenz Julienne]

The resulting code is way more readeable and intuitive compared to plain
list_for_each.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_arm.c   | 52 ++-
 1 file changed, 16 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 835152799433..76de36e9e3b6 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -280,16 +280,11 @@ VCHIQ_STATUS_T vchiq_shutdown(VCHIQ_INSTANCE_T instance)
"%s(%p): returning %d", __func__, instance, status);
 
if (status == VCHIQ_SUCCESS) {
-   struct list_head *pos, *next;
+   struct bulk_waiter_node *waiter, *next;
 
-   list_for_each_safe(pos, next,
-   &instance->bulk_waiter_list) {
-   struct bulk_waiter_node *waiter;
-
-   waiter = list_entry(pos,
-   struct bulk_waiter_node,
-   list);
-   list_del(pos);
+   list_for_each_entry_safe(waiter, next,
+&instance->bulk_waiter_list, list) {
+   list_del(&waiter->list);
vchiq_log_info(vchiq_arm_log_level,
"bulk_waiter - cleaned up %pK for pid 
%d",
waiter, waiter->pid);
@@ -473,7 +468,6 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle, 
void *data,
VCHIQ_SERVICE_T *service;
VCHIQ_STATUS_T status;
struct bulk_waiter_node *waiter = NULL;
-   struct list_head *pos;
 
service = find_service_by_handle(handle);
if (!service)
@@ -484,13 +478,9 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T 
handle, void *data,
unlock_service(service);
 
mutex_lock(&instance->bulk_waiter_list_mutex);
-   list_for_each(pos, &instance->bulk_waiter_list) {
-   if (list_entry(pos, struct bulk_waiter_node,
-   list)->pid == current->pid) {
-   waiter = list_entry(pos,
-   struct bulk_waiter_node,
-   list);
-   list_del(pos);
+   list_for_each_entry(waiter, &instance->bulk_waiter_list, list) {
+   if (waiter->pid == current->pid) {
+   list_del(&waiter->list);
break;
}
}
@@ -1135,21 +1125,16 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
unsigned long arg)
ret = -ENOMEM;
break;
}
+
args.userdata = &waiter->bulk_waiter;
} else if (args.mode == VCHIQ_BULK_MODE_WAITING) {
-   struct list_head *pos;
-
mutex_lock(&instance->bulk_waiter_list_mutex);
-   list_for_each(pos, &instance->bulk_waiter_list) {
-   if (list_entry(pos, struct bulk_waiter_node,
-   list)->pid == current->pid) {
-   waiter = list_entry(pos,
-   struct bulk_waiter_node,
-   list);
-   list_del(pos);
+   list_for_each_entry(waiter, &instance->bulk_waiter_list,
+   list) {
+   if (waiter->pid == current->pid) {
+   list_del(&waiter->list);
break;
}
-
}
mutex_unlock(&instance->bulk_waiter_list_mutex);
if (!waiter) {
@@ -2163,16 +2148,11 @@ vchiq_release(struct inode *inode, struct file *file)
vchiq_release_internal(instance->state, NULL);
 
{
-   struct list_head *pos, *next;
-
-   list_for_each_safe(pos, next,
-   &instance->bulk_waiter_list) {
-   struct bulk_waiter_node *waiter;
+   struct bulk_waiter_node *waiter, *next;
 
-   waiter = list_entry(pos,
-   struct bulk_waiter_node,
-   list);
-   list_del(pos);
+   list_for_each_entry_safe(waiter, next,
+   &instance->bulk_waiter_list, list) {
+   list_del(&waiter->list

[PATCH 09/16] staging: vchiq_core: do not initialize semaphores twice

[Nicolas Saenz Julienne]

vchiq_init_state() initialises a series of semaphores to then call
remote_event_create() on the same semaphores, which initializes them
again. We get rid of the second initialization.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c| 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index dee5ea7bfe4f..8b23ea5322e8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -418,12 +418,11 @@ vchiq_set_conn_state(VCHIQ_STATE_T *state, 
VCHIQ_CONNSTATE_T newstate)
 }
 
 static inline void
-remote_event_create(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
+remote_event_create(REMOTE_EVENT_T *event)
 {
event->armed = 0;
/* Don't clear the 'fired' flag because it may already have been set
** by the other side. */
-   sema_init((struct semaphore *)((char *)state + event->event), 0);
 }
 
 static inline int
@@ -2237,18 +2236,18 @@ vchiq_init_state(VCHIQ_STATE_T *state, 
VCHIQ_SLOT_ZERO_T *slot_zero)
state->data_quota = state->slot_queue_available - 1;
 
local->trigger.event = offsetof(VCHIQ_STATE_T, trigger_event);
-   remote_event_create(state, &local->trigger);
+   remote_event_create(&local->trigger);
local->tx_pos = 0;
 
local->recycle.event = offsetof(VCHIQ_STATE_T, recycle_event);
-   remote_event_create(state, &local->recycle);
+   remote_event_create(&local->recycle);
local->slot_queue_recycle = state->slot_queue_available;
 
local->sync_trigger.event = offsetof(VCHIQ_STATE_T, sync_trigger_event);
-   remote_event_create(state, &local->sync_trigger);
+   remote_event_create(&local->sync_trigger);
 
local->sync_release.event = offsetof(VCHIQ_STATE_T, sync_release_event);
-   remote_event_create(state, &local->sync_release);
+   remote_event_create(&local->sync_release);
 
/* At start-of-day, the slot is empty and available */
((VCHIQ_HEADER_T *)SLOT_DATA_FROM_INDEX(state, local->slot_sync))->msgid
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 12/16] staging: vchiq_util: get rid of unneeded memory barriers

[Nicolas Saenz Julienne]

All the memory operations featured in this file modify/access memory
that is only accessed by the CPU. So we can assume that all the memory
barrier handling done by the completion routines is good enough for us.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_util.c  | 32 ---
 1 file changed, 32 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
index 44b954daa74a..4b8554bc647e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c
@@ -84,20 +84,7 @@ void vchiu_queue_push(VCHIU_QUEUE_T *queue, VCHIQ_HEADER_T 
*header)
flush_signals(current);
}
 
-   /*
-* Write to queue->storage must be visible after read from
-* queue->read
-*/
-   smp_mb();
-
queue->storage[queue->write & (queue->size - 1)] = header;
-
-   /*
-* Write to queue->storage must be visible before write to
-* queue->write
-*/
-   smp_wmb();
-
queue->write++;
 
complete(&queue->push);
@@ -112,12 +99,6 @@ VCHIQ_HEADER_T *vchiu_queue_peek(VCHIU_QUEUE_T *queue)
 
complete(&queue->push); // We haven't removed anything from the queue.
 
-   /*
-* Read from queue->storage must be visible after read from
-* queue->write
-*/
-   smp_rmb();
-
return queue->storage[queue->read & (queue->size - 1)];
 }
 
@@ -130,20 +111,7 @@ VCHIQ_HEADER_T *vchiu_queue_pop(VCHIU_QUEUE_T *queue)
flush_signals(current);
}
 
-   /*
-* Read from queue->storage must be visible after read from
-* queue->write
-*/
-   smp_rmb();
-
header = queue->storage[queue->read & (queue->size - 1)];
-
-   /*
-* Read from queue->storage must be visible before write to
-* queue->read
-*/
-   smp_mb();
-
queue->read++;
 
complete(&queue->pop);
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 14/16] staging: vchiq_arm: rework probe and init functions

[Nicolas Saenz Julienne]

Moves the allocation of a chardev region and class creation to the init
function of the driver since those functions are meant to be run on a
per driver basis, as opposed to the code run in the probe function which
is run in a per device basis.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_arm.c   | 71 ---
 1 file changed, 45 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 20359924ed45..03cc6947c03c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -166,7 +166,6 @@ static struct cdevvchiq_cdev;
 static dev_t  vchiq_devid;
 static VCHIQ_STATE_T g_state;
 static struct class  *vchiq_class;
-static struct device *vchiq_dev;
 static DEFINE_SPINLOCK(msg_queue_spinlock);
 static struct platform_device *bcm2835_camera;
 
@@ -3557,34 +3556,19 @@ static int vchiq_probe(struct platform_device *pdev)
if (err != 0)
goto failed_platform_init;
 
-   err = alloc_chrdev_region(&vchiq_devid, VCHIQ_MINOR, 1, DEVICE_NAME);
-   if (err != 0) {
-   vchiq_log_error(vchiq_arm_log_level,
-   "Unable to allocate device number");
-   goto failed_platform_init;
-   }
cdev_init(&vchiq_cdev, &vchiq_fops);
vchiq_cdev.owner = THIS_MODULE;
err = cdev_add(&vchiq_cdev, vchiq_devid, 1);
if (err != 0) {
vchiq_log_error(vchiq_arm_log_level,
"Unable to register device");
-   goto failed_cdev_add;
+   goto failed_platform_init;
}
 
-   /* create sysfs entries */
-   vchiq_class = class_create(THIS_MODULE, DEVICE_NAME);
-   err = PTR_ERR(vchiq_class);
-   if (IS_ERR(vchiq_class))
-   goto failed_class_create;
-
-   vchiq_dev = device_create(vchiq_class, NULL,
-   vchiq_devid, NULL, "vchiq");
-   err = PTR_ERR(vchiq_dev);
-   if (IS_ERR(vchiq_dev))
+   if (IS_ERR(device_create(vchiq_class, &pdev->dev, vchiq_devid,
+NULL, "vchiq")))
goto failed_device_create;
 
-   /* create debugfs entries */
vchiq_debugfs_init();
 
vchiq_log_info(vchiq_arm_log_level,
@@ -3599,11 +3583,7 @@ static int vchiq_probe(struct platform_device *pdev)
return 0;
 
 failed_device_create:
-   class_destroy(vchiq_class);
-failed_class_create:
cdev_del(&vchiq_cdev);
-failed_cdev_add:
-   unregister_chrdev_region(vchiq_devid, 1);
 failed_platform_init:
vchiq_log_warning(vchiq_arm_log_level, "could not load vchiq");
return err;
@@ -3614,9 +3594,7 @@ static int vchiq_remove(struct platform_device *pdev)
platform_device_unregister(bcm2835_camera);
vchiq_debugfs_deinit();
device_destroy(vchiq_class, vchiq_devid);
-   class_destroy(vchiq_class);
cdev_del(&vchiq_cdev);
-   unregister_chrdev_region(vchiq_devid, 1);
 
return 0;
 }
@@ -3629,7 +3607,48 @@ static struct platform_driver vchiq_driver = {
.probe = vchiq_probe,
.remove = vchiq_remove,
 };
-module_platform_driver(vchiq_driver);
+
+static int __init vchiq_driver_init(void)
+{
+   int ret;
+
+   vchiq_class = class_create(THIS_MODULE, DEVICE_NAME);
+   if (IS_ERR(vchiq_class)) {
+   pr_err("Failed to create vchiq class\n");
+   return PTR_ERR(vchiq_class);
+   }
+
+   ret = alloc_chrdev_region(&vchiq_devid, VCHIQ_MINOR, 1, DEVICE_NAME);
+   if (ret) {
+   pr_err("Failed to allocate vchiq's chrdev region\n");
+   goto class_destroy;
+   }
+
+   ret = platform_driver_register(&vchiq_driver);
+   if (ret) {
+   pr_err("Failed to register vchiq driver\n");
+   goto region_unregister;
+   }
+
+   return 0;
+
+region_unregister:
+   platform_driver_unregister(&vchiq_driver);
+
+class_destroy:
+   class_destroy(vchiq_class);
+
+   return ret;
+}
+module_init(vchiq_driver_init);
+
+static void __exit vchiq_driver_exit(void)
+{
+   platform_driver_unregister(&vchiq_driver);
+   unregister_chrdev_region(vchiq_devid, 1);
+   class_destroy(vchiq_class);
+}
+module_exit(vchiq_driver_exit);
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("Videocore VCHIQ driver");
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 08/16] staging: vchiq_core: remove unnecessary safety checks in vchiq_init_state

[Nicolas Saenz Julienne]

vchiq_init_state() checks the initial contents of slot_zero are correct.
These are set in vchiq_init_slots(), using the same hard-coded defaults
as the checks. Both functions are called sequentially and Video Core
isn't yet aware of the slot's address. There is no way the contents of
slot_zero changed in between functions, making the checks useless.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_core.c  | 59 ---
 1 file changed, 59 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 34a892011296..dee5ea7bfe4f 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2170,65 +2170,6 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T 
*slot_zero)
return VCHIQ_ERROR;
}
 
-   /* Check the input configuration */
-
-   if (slot_zero->magic != VCHIQ_MAGIC) {
-   vchiq_loud_error_header();
-   vchiq_loud_error("Invalid VCHIQ magic value found.");
-   vchiq_loud_error("slot_zero=%pK: magic=%x (expected %x)",
-   slot_zero, slot_zero->magic, VCHIQ_MAGIC);
-   vchiq_loud_error_footer();
-   return VCHIQ_ERROR;
-   }
-
-   if (slot_zero->version < VCHIQ_VERSION_MIN) {
-   vchiq_loud_error_header();
-   vchiq_loud_error("Incompatible VCHIQ versions found.");
-   vchiq_loud_error("slot_zero=%pK: VideoCore version=%d (minimum 
%d)",
-   slot_zero, slot_zero->version, VCHIQ_VERSION_MIN);
-   vchiq_loud_error("Restart with a newer VideoCore image.");
-   vchiq_loud_error_footer();
-   return VCHIQ_ERROR;
-   }
-
-   if (VCHIQ_VERSION < slot_zero->version_min) {
-   vchiq_loud_error_header();
-   vchiq_loud_error("Incompatible VCHIQ versions found.");
-   vchiq_loud_error("slot_zero=%pK: version=%d (VideoCore minimum 
%d)",
-   slot_zero, VCHIQ_VERSION, slot_zero->version_min);
-   vchiq_loud_error("Restart with a newer kernel.");
-   vchiq_loud_error_footer();
-   return VCHIQ_ERROR;
-   }
-
-   if ((slot_zero->slot_zero_size != sizeof(VCHIQ_SLOT_ZERO_T)) ||
-(slot_zero->slot_size != VCHIQ_SLOT_SIZE) ||
-(slot_zero->max_slots != VCHIQ_MAX_SLOTS) ||
-(slot_zero->max_slots_per_side != VCHIQ_MAX_SLOTS_PER_SIDE)) {
-   vchiq_loud_error_header();
-   if (slot_zero->slot_zero_size != sizeof(VCHIQ_SLOT_ZERO_T))
-   vchiq_loud_error("slot_zero=%pK: slot_zero_size=%d 
(expected %d)",
-   slot_zero, slot_zero->slot_zero_size,
-   (int)sizeof(VCHIQ_SLOT_ZERO_T));
-   if (slot_zero->slot_size != VCHIQ_SLOT_SIZE)
-   vchiq_loud_error("slot_zero=%pK: slot_size=%d (expected 
%d)",
-   slot_zero, slot_zero->slot_size,
-   VCHIQ_SLOT_SIZE);
-   if (slot_zero->max_slots != VCHIQ_MAX_SLOTS)
-   vchiq_loud_error("slot_zero=%pK: max_slots=%d (expected 
%d)",
-   slot_zero, slot_zero->max_slots,
-   VCHIQ_MAX_SLOTS);
-   if (slot_zero->max_slots_per_side != VCHIQ_MAX_SLOTS_PER_SIDE)
-   vchiq_loud_error("slot_zero=%pK: max_slots_per_side=%d 
(expected %d)",
-   slot_zero, slot_zero->max_slots_per_side,
-   VCHIQ_MAX_SLOTS_PER_SIDE);
-   vchiq_loud_error_footer();
-   return VCHIQ_ERROR;
-   }
-
-   if (VCHIQ_VERSION < slot_zero->version)
-   slot_zero->version = VCHIQ_VERSION;
-
local = &slot_zero->slave;
remote = &slot_zero->master;
 
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/16] staging: vchiq-core: get rid of is_master distinction

[Nicolas Saenz Julienne]

VCHIQ bulk transfers are what most people call DMA transfers. The CPU
sends a list of physical addresses to the VideoCore which then access
the memory directly without the need for CPU interaction.  With this
setup we call the CPU the "slave" and the VideoCore the "master".

There seems to be an option to switch roles in vchiq. Which nobody is
using nor is properly implemented. So we get rid of the "is_master == 1"
option, and all the related code.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_2835_arm.c  |  12 +-
 .../interface/vchiq_arm/vchiq_core.c  | 300 +++---
 .../interface/vchiq_arm/vchiq_core.h  |  11 +-
 3 files changed, 38 insertions(+), 285 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 014583cdf367..ecee54a31f8d 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -163,7 +163,7 @@ int vchiq_platform_init(struct platform_device *pdev, 
VCHIQ_STATE_T *state)
*(char **)&g_fragments_base[i * g_fragments_size] = NULL;
sema_init(&g_free_fragments_sema, MAX_FRAGMENTS);
 
-   if (vchiq_init_state(state, vchiq_slot_zero, 0) != VCHIQ_SUCCESS)
+   if (vchiq_init_state(state, vchiq_slot_zero) != VCHIQ_SUCCESS)
return -EINVAL;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -278,16 +278,6 @@ vchiq_complete_bulk(VCHIQ_BULK_T *bulk)
  bulk->actual);
 }
 
-void
-vchiq_transfer_bulk(VCHIQ_BULK_T *bulk)
-{
-   /*
-* This should only be called on the master (VideoCore) side, but
-* provide an implementation to avoid the need for ifdefery.
-*/
-   BUG();
-}
-
 void
 vchiq_dump_platform_state(void *dump_context)
 {
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 8c7bda2e7cb6..34a892011296 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -85,8 +85,6 @@ int vchiq_core_log_level = VCHIQ_LOG_DEFAULT;
 int vchiq_core_msg_log_level = VCHIQ_LOG_DEFAULT;
 int vchiq_sync_log_level = VCHIQ_LOG_DEFAULT;
 
-static atomic_t pause_bulks_count = ATOMIC_INIT(0);
-
 static DEFINE_SPINLOCK(service_spinlock);
 DEFINE_SPINLOCK(bulk_waiter_spinlock);
 static DEFINE_SPINLOCK(quota_spinlock);
@@ -1222,32 +1220,7 @@ notify_bulks(VCHIQ_SERVICE_T *service, 
VCHIQ_BULK_QUEUE_T *queue,
(queue == &service->bulk_tx) ? 't' : 'r',
queue->process, queue->remote_notify, queue->remove);
 
-   if (service->state->is_master) {
-   while (queue->remote_notify != queue->process) {
-   VCHIQ_BULK_T *bulk =
-   &queue->bulks[BULK_INDEX(queue->remote_notify)];
-   int msgtype = (bulk->dir == VCHIQ_BULK_TRANSMIT) ?
-   VCHIQ_MSG_BULK_RX_DONE : VCHIQ_MSG_BULK_TX_DONE;
-   int msgid = VCHIQ_MAKE_MSG(msgtype, service->localport,
-   service->remoteport);
-   /* Only reply to non-dummy bulk requests */
-   if (bulk->remote_data) {
-   status = queue_message(
-   service->state,
-   NULL,
-   msgid,
-   memcpy_copy_callback,
-   &bulk->actual,
-   4,
-   0);
-   if (status != VCHIQ_SUCCESS)
-   break;
-   }
-   queue->remote_notify++;
-   }
-   } else {
-   queue->remote_notify = queue->process;
-   }
+   queue->remote_notify = queue->process;
 
if (status == VCHIQ_SUCCESS) {
while (queue->remove != queue->remote_notify) {
@@ -1385,63 +1358,6 @@ poll_services(VCHIQ_STATE_T *state)
}
 }
 
-/* Called by the slot handler or application threads, holding the bulk mutex. 
*/
-static int
-resolve_bulks(VCHIQ_SERVICE_T *service, VCHIQ_BULK_QUEUE_T *queue)
-{
-   VCHIQ_STATE_T *state = service->state;
-   int resolved = 0;
-
-   while ((queue->process != queue->local_insert) &&
-   (queue->process != queue->remote_insert)) {
-   VCHIQ_BULK_T *bulk = &queue->bulks[BULK_INDEX(queue->process)];
-
-   vchiq_log_trace(vchiq_core_log_level,
-   "%d: rb:%d %cx - li=%x ri=%x p=%x",
-  

[PATCH 13/16] staging: vchiq_core: fix logic redundancy in parse_open

[Nicolas Saenz Julienne]

We update sync to reflect that the firmware version is compatible with
that option. We don't need to check both of them again further down the
code.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c| 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index a45cdd08e209..5ee667d46eb5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1461,9 +1461,7 @@ parse_open(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header)
service->sync = 0;
 
/* Acknowledge the OPEN */
-   if (service->sync &&
-   (state->version_common >=
-VCHIQ_VERSION_SYNCHRONOUS_MODE)) {
+   if (service->sync) {
if (queue_message_sync(
state,
NULL,
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/16] staging: vchiq_arm: rework close/remove_service IOCTLS

[Nicolas Saenz Julienne]

The implementation of both IOCTLS was the same except for one function
call. This joins both implementations and updates the code to avoid
unneeded indentations.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_arm.c   | 66 +++
 1 file changed, 24 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 5af3f2651bd3..835152799433 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1019,55 +1019,37 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
unsigned long arg)
}
} break;
 
-   case VCHIQ_IOC_CLOSE_SERVICE: {
+   case VCHIQ_IOC_CLOSE_SERVICE:
+   case VCHIQ_IOC_REMOVE_SERVICE: {
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
+   USER_SERVICE_T *user_service;
 
service = find_service_for_instance(instance, handle);
-   if (service != NULL) {
-   USER_SERVICE_T *user_service =
-   (USER_SERVICE_T *)service->base.userdata;
-   /* close_pending is false on first entry, and when the
-  wait in vchiq_close_service has been interrupted. */
-   if (!user_service->close_pending) {
-   status = vchiq_close_service(service->handle);
-   if (status != VCHIQ_SUCCESS)
-   break;
-   }
-
-   /* close_pending is true once the underlying service
-  has been closed until the client library calls the
-  CLOSE_DELIVERED ioctl, signalling close_event. */
-   if (user_service->close_pending &&
-   down_interruptible(&user_service->close_event))
-   status = VCHIQ_RETRY;
-   } else
+   if (!service) {
ret = -EINVAL;
-   } break;
+   break;
+   }
 
-   case VCHIQ_IOC_REMOVE_SERVICE: {
-   VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
+   user_service = service->base.userdata;
 
-   service = find_service_for_instance(instance, handle);
-   if (service != NULL) {
-   USER_SERVICE_T *user_service =
-   (USER_SERVICE_T *)service->base.userdata;
-   /* close_pending is false on first entry, and when the
-  wait in vchiq_close_service has been interrupted. */
-   if (!user_service->close_pending) {
-   status = vchiq_remove_service(service->handle);
-   if (status != VCHIQ_SUCCESS)
-   break;
-   }
+   /* close_pending is false on first entry, and when the
+  wait in vchiq_close_service has been interrupted. */
+   if (!user_service->close_pending) {
+   status = (cmd == VCHIQ_IOC_CLOSE_SERVICE) ?
+vchiq_close_service(service->handle) :
+vchiq_remove_service(service->handle);
+   if (status != VCHIQ_SUCCESS)
+   break;
+   }
 
-   /* close_pending is true once the underlying service
-  has been closed until the client library calls the
-  CLOSE_DELIVERED ioctl, signalling close_event. */
-   if (user_service->close_pending &&
-   down_interruptible(&user_service->close_event))
-   status = VCHIQ_RETRY;
-   } else
-   ret = -EINVAL;
-   } break;
+   /* close_pending is true once the underlying service
+  has been closed until the client library calls the
+  CLOSE_DELIVERED ioctl, signalling close_event. */
+   if (user_service->close_pending &&
+   down_interruptible(&user_service->close_event))
+   status = VCHIQ_RETRY;
+   break;
+   }
 
case VCHIQ_IOC_USE_SERVICE:
case VCHIQ_IOC_RELEASE_SERVICE: {
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/16] staging: vchiq_arm: rework vchiq_ioc_copy_element_data

[Nicolas Saenz Julienne]

The function is passed to vchiq_core.c for it to go trough all the
transfer elements (an array of pointers to data) and copy them into the
actual transfer memory (contiguous memory).

The logic in the function was "copy an element and return, except when
the element is empty, in which case look for the next non-empty element
and copy it. The function will be called as many times as necessary until
all the elements are copied".

Now, this approach already forces the function to loop around elements
and felt convoluted, so it was changed to a more straightforward "Copy
all the elements into memory as long as they fit".

The resulting function is shorter and simpler.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_arm.c   | 89 +++
 1 file changed, 31 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 525458551a22..7fa734991db9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -752,74 +752,48 @@ static void close_delivered(USER_SERVICE_T *user_service)
 }
 
 struct vchiq_io_copy_callback_context {
-   struct vchiq_element *current_element;
-   size_t current_element_offset;
+   struct vchiq_element *element;
+   size_t element_offset;
unsigned long elements_to_go;
-   size_t current_offset;
 };
 
-static ssize_t
-vchiq_ioc_copy_element_data(
-   void *context,
-   void *dest,
-   size_t offset,
-   size_t maxsize)
+static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
+  size_t offset, size_t maxsize)
 {
-   long res;
+   struct vchiq_io_copy_callback_context *cc = context;
+   size_t total_bytes_copied = 0;
size_t bytes_this_round;
-   struct vchiq_io_copy_callback_context *copy_context =
-   (struct vchiq_io_copy_callback_context *)context;
-
-   if (offset != copy_context->current_offset)
-   return 0;
-
-   if (!copy_context->elements_to_go)
-   return 0;
-
-   /*
-* Complex logic here to handle the case of 0 size elements
-* in the middle of the array of elements.
-*
-* Need to skip over these 0 size elements.
-*/
-   while (1) {
-   bytes_this_round = min(copy_context->current_element->size -
-  copy_context->current_element_offset,
-  maxsize);
-
-   if (bytes_this_round)
-   break;
 
-   copy_context->elements_to_go--;
-   copy_context->current_element++;
-   copy_context->current_element_offset = 0;
+   while (total_bytes_copied < maxsize) {
+   if (!cc->elements_to_go)
+   return total_bytes_copied;
 
-   if (!copy_context->elements_to_go)
-   return 0;
-   }
+   if (!cc->element->size) {
+   cc->elements_to_go--;
+   cc->element++;
+   cc->element_offset = 0;
+   continue;
+   }
 
-   res = copy_from_user(dest,
-copy_context->current_element->data +
-copy_context->current_element_offset,
-bytes_this_round);
+   bytes_this_round = min(cc->element->size - cc->element_offset,
+  maxsize - total_bytes_copied);
 
-   if (res != 0)
-   return -EFAULT;
+   if (copy_from_user(dest + total_bytes_copied,
+ cc->element->data + cc->element_offset,
+ bytes_this_round))
+   return -EFAULT;
 
-   copy_context->current_element_offset += bytes_this_round;
-   copy_context->current_offset += bytes_this_round;
+   cc->element_offset += bytes_this_round;
+   total_bytes_copied += bytes_this_round;
 
-   /*
-* Check if done with current element, and if so advance to the next.
-*/
-   if (copy_context->current_element_offset ==
-   copy_context->current_element->size) {
-   copy_context->elements_to_go--;
-   copy_context->current_element++;
-   copy_context->current_element_offset = 0;
+   if (cc->element_offset == cc->element->size) {
+   cc->elements_to_go--;
+   cc->element++;
+   cc->element_offset = 0;
+   }
}
 
-   return bytes_this_round;
+   return maxsize;
 }
 
 /**
@@ -836,10 +810,9 @@ vchiq_ioc_qu

[PATCH 10/16] staging: vchiq_core: don't add a wmb() before remote_event_signal()

[Nicolas Saenz Julienne]

It's the first thing remote_event_signal() does.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c| 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 8b23ea5322e8..5791c2b670fa 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1137,9 +1137,6 @@ queue_message_sync(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T 
*service,
size);
}
 
-   /* Make sure the new header is visible to the peer. */
-   wmb();
-
remote_event_signal(&state->remote->sync_trigger);
 
if (VCHIQ_MSG_TYPE(msgid) != VCHIQ_MSG_PAUSE)
@@ -3269,7 +3266,6 @@ static void
 release_message_sync(VCHIQ_STATE_T *state, VCHIQ_HEADER_T *header)
 {
header->msgid = VCHIQ_MSGID_PADDING;
-   wmb();
remote_event_signal(&state->remote->sync_release);
 }
 
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/16] staging: vchiq: use completions instead of semaphores

[Nicolas Saenz Julienne]

It is preferred in the kernel to avoid using semaphores to wait for
events, as they are optimised for the opposite situation; where the
common case is that they are available and may block only occasionally.
FYI see this thread: https://lkml.org/lkml/2008/4/11/323.

Also completions are semantically more explicit in this case.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_arm.c   |  60 ++-
 .../interface/vchiq_arm/vchiq_core.c  | 100 +-
 .../interface/vchiq_arm/vchiq_core.h  |  26 ++---
 .../interface/vchiq_arm/vchiq_util.c  |  16 +--
 .../interface/vchiq_arm/vchiq_util.h  |   6 +-
 5 files changed, 106 insertions(+), 102 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 7fa734991db9..20359924ed45 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -44,7 +44,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -121,9 +121,9 @@ typedef struct user_service_struct {
int message_available_pos;
int msg_insert;
int msg_remove;
-   struct semaphore insert_event;
-   struct semaphore remove_event;
-   struct semaphore close_event;
+   struct completion insert_event;
+   struct completion remove_event;
+   struct completion close_event;
VCHIQ_HEADER_T * msg_queue[MSG_QUEUE_SIZE];
 } USER_SERVICE_T;
 
@@ -138,8 +138,8 @@ struct vchiq_instance_struct {
VCHIQ_COMPLETION_DATA_T completions[MAX_COMPLETIONS];
int completion_insert;
int completion_remove;
-   struct semaphore insert_event;
-   struct semaphore remove_event;
+   struct completion insert_event;
+   struct completion remove_event;
struct mutex completion_mutex;
 
int connected;
@@ -562,7 +562,8 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T 
reason,
vchiq_log_trace(vchiq_arm_log_level,
"%s - completion queue full", __func__);
DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
-   if (down_interruptible(&instance->remove_event) != 0) {
+   if (wait_for_completion_interruptible(
+   &instance->remove_event)) {
vchiq_log_info(vchiq_arm_log_level,
"service_callback interrupted");
return VCHIQ_RETRY;
@@ -600,7 +601,7 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T 
reason,
insert++;
instance->completion_insert = insert;
 
-   up(&instance->insert_event);
+   complete(&instance->insert_event);
 
return VCHIQ_SUCCESS;
 }
@@ -673,7 +674,8 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T 
*header,
}
 
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
-   if (down_interruptible(&user_service->remove_event)
+   if (wait_for_completion_interruptible(
+   &user_service->remove_event)
!= 0) {
vchiq_log_info(vchiq_arm_log_level,
"%s interrupted", __func__);
@@ -705,7 +707,7 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T 
*header,
}
 
spin_unlock(&msg_queue_spinlock);
-   up(&user_service->insert_event);
+   complete(&user_service->insert_event);
 
header = NULL;
}
@@ -745,7 +747,7 @@ static void close_delivered(USER_SERVICE_T *user_service)
unlock_service(user_service->service);
 
/* Wake the user-thread blocked in close_ or remove_service */
-   up(&user_service->close_event);
+   complete(&user_service->close_event);
 
user_service->close_pending = 0;
}
@@ -867,7 +869,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
long arg)
if (status == VCHIQ_SUCCESS) {
/* Wake the completion thread and ask it to exit */
instance->closing = 1;
-   up(&instance->insert_event);
+   complete(&instance->insert_event);
}
 
break;
@@ -948,9 +950,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned 
long arg)
instance->completion_remove - 1;
user_service->msg_insert = 0;
user_service->msg_remove = 0;
-   sema_init(&user_service->insert_event, 0);
-   sema_init(&user_service->remove_event, 0);
-   sema_init(&user_service

[PATCH 03/16] staging: vchiq_shim: delete vchi_service_create

[Nicolas Saenz Julienne]

No one is using the API neither in the actual staging tree nor in the
downstream tree (https://github.com/raspberrypi/linux).

Signed-off-by: Nicolas Saenz Julienne 
---
 .../vc04_services/interface/vchi/vchi.h   |  5 ---
 .../interface/vchiq_arm/vchiq_shim.c  | 32 ---
 2 files changed, 37 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchi/vchi.h 
b/drivers/staging/vc04_services/interface/vchi/vchi.h
index 01381904775d..379a16ebfd5b 100644
--- a/drivers/staging/vc04_services/interface/vchi/vchi.h
+++ b/drivers/staging/vc04_services/interface/vchi/vchi.h
@@ -113,11 +113,6 @@ extern uint32_t vchi_current_time(VCHI_INSTANCE_T 
instance_handle);
 /**
  Global service API
  */
-// Routine to create a named service
-extern int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle,
-  SERVICE_CREATION_T *setup,
-  VCHI_SERVICE_HANDLE_T *handle);
-
 // Routine to destroy a service
 extern int32_t vchi_service_destroy(const VCHI_SERVICE_HANDLE_T handle);
 
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
index c3223fcdaf87..81cac68f4b78 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
@@ -660,38 +660,6 @@ int32_t vchi_service_open(VCHI_INSTANCE_T instance_handle,
 }
 EXPORT_SYMBOL(vchi_service_open);
 
-int32_t vchi_service_create(VCHI_INSTANCE_T instance_handle,
-   SERVICE_CREATION_T *setup,
-   VCHI_SERVICE_HANDLE_T *handle)
-{
-   VCHIQ_INSTANCE_T instance = (VCHIQ_INSTANCE_T)instance_handle;
-   struct shim_service *service = service_alloc(instance, setup);
-
-   *handle = (VCHI_SERVICE_HANDLE_T)service;
-
-   if (service) {
-   VCHIQ_SERVICE_PARAMS_T params;
-   VCHIQ_STATUS_T status;
-
-   memset(¶ms, 0, sizeof(params));
-   params.fourcc = setup->service_id;
-   params.callback = shim_callback;
-   params.userdata = service;
-   params.version = setup->version.version;
-   params.version_min = setup->version.version_min;
-   status = vchiq_add_service(instance, ¶ms, &service->handle);
-
-   if (status != VCHIQ_SUCCESS) {
-   service_free(service);
-   service = NULL;
-   *handle = NULL;
-   }
-   }
-
-   return (service != NULL) ? 0 : -1;
-}
-EXPORT_SYMBOL(vchi_service_create);
-
 int32_t vchi_service_close(const VCHI_SERVICE_HANDLE_T handle)
 {
int32_t ret = -1;
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 16/16] staging: vchiq: add more tasks to the TODO list

[Nicolas Saenz Julienne]

The TODO list was missing some tasks needed before upstreaming the
device.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../staging/vc04_services/interface/vchi/TODO | 42 +++
 1 file changed, 42 insertions(+)

diff --git a/drivers/staging/vc04_services/interface/vchi/TODO 
b/drivers/staging/vc04_services/interface/vchi/TODO
index 0b3ec75ff627..fc2752bc95b2 100644
--- a/drivers/staging/vc04_services/interface/vchi/TODO
+++ b/drivers/staging/vc04_services/interface/vchi/TODO
@@ -49,3 +49,45 @@ such as dev_info, dev_dbg, and friends.
 
 A short top-down description of this driver's architecture (function of
 kthreads, userspace, limitations) could be very helpful for reviewers.
+
+7) Review and comment memory barriers
+
+There is a heavy use of memory barriers in this driver, it would be very
+beneficial to go over all of them and, if correct, comment on their merits.
+Extra points to whomever confidently reviews the remote_event_*() family of
+functions.
+
+8) Get rid of custom function return values
+
+Most functions use a custom set of return values, we should force proper Linux
+error numbers. Special care is needed for VCHIQ_RETRY.
+
+9) Reformat core code with more sane indentations
+
+The code follows the 80 characters limitation yet tends to go 3 or 4 levels of
+indentation deep making it very unpleasant to read. This is specially relevant
+in the character driver ioctl code and in the core thread functions.
+
+10) Reorganize file structure: Move char driver to it's own file and join both
+platform files
+
+The cdev is defined alongside with the platform code in vchiq_arm.c. It would
+be nice to completely decouple it from the actual core code. For instance to be
+able to use bcm2835-audio without having /dev/vchiq created. One could argue
+it's better for security reasons or general cleanliness. It could even be
+interesting to create two different kernel modules, something the likes of
+vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process.
+
+The code in vchiq_bcm2835_arm.c should fit in the generic platform file.
+
+12) Get rid of all the struct typedefs
+
+Most structs are typedefd, it's not encouraged in the kernel.
+
+13) Get rid of all non essential global structures and create a proper per
+device structure
+
+The first thing one generally sees in a probe function is a memory allocation
+for all the device specific data. This structure is then passed all over the
+driver. This is good practice since it makes the driver work regardless of the
+number of devices probed.
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 15/16] staging: vchiq_arm: fix open/release cdev functions

[Nicolas Saenz Julienne]

Both functions checked the minor number of the cdev prior running the
code. This was useless since the number of devices is already limited by
alloc_chrdev_region.

This removes the check and reindents the code where relevant.

Signed-off-by: Nicolas Saenz Julienne 
---
 .../interface/vchiq_arm/vchiq_arm.c   | 247 +++---
 1 file changed, 100 insertions(+), 147 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 03cc6947c03c..0caee2d6946f 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -63,8 +63,6 @@
 #undef MODULE_PARAM_PREFIX
 #define MODULE_PARAM_PREFIX DEVICE_NAME "."
 
-#define VCHIQ_MINOR 0
-
 /* Some per-instance constants */
 #define MAX_COMPLETIONS 128
 #define MAX_SERVICES 64
@@ -1955,195 +1953,150 @@ vchiq_compat_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
 
 #endif
 
-/
-*
-*   vchiq_open
-*
-***/
-
-static int
-vchiq_open(struct inode *inode, struct file *file)
+static int vchiq_open(struct inode *inode, struct file *file)
 {
-   int dev = iminor(inode) & 0x0f;
+   VCHIQ_STATE_T *state = vchiq_get_state();
+   VCHIQ_INSTANCE_T instance;
 
vchiq_log_info(vchiq_arm_log_level, "vchiq_open");
-   switch (dev) {
-   case VCHIQ_MINOR: {
-   VCHIQ_STATE_T *state = vchiq_get_state();
-   VCHIQ_INSTANCE_T instance;
 
-   if (!state) {
-   vchiq_log_error(vchiq_arm_log_level,
+   if (!state) {
+   vchiq_log_error(vchiq_arm_log_level,
"vchiq has no connection to VideoCore");
-   return -ENOTCONN;
-   }
-
-   instance = kzalloc(sizeof(*instance), GFP_KERNEL);
-   if (!instance)
-   return -ENOMEM;
+   return -ENOTCONN;
+   }
 
-   instance->state = state;
-   instance->pid = current->tgid;
+   instance = kzalloc(sizeof(*instance), GFP_KERNEL);
+   if (!instance)
+   return -ENOMEM;
 
-   vchiq_debugfs_add_instance(instance);
+   instance->state = state;
+   instance->pid = current->tgid;
 
-   init_completion(&instance->insert_event);
-   init_completion(&instance->remove_event);
-   mutex_init(&instance->completion_mutex);
-   mutex_init(&instance->bulk_waiter_list_mutex);
-   INIT_LIST_HEAD(&instance->bulk_waiter_list);
+   vchiq_debugfs_add_instance(instance);
 
-   file->private_data = instance;
-   } break;
+   init_completion(&instance->insert_event);
+   init_completion(&instance->remove_event);
+   mutex_init(&instance->completion_mutex);
+   mutex_init(&instance->bulk_waiter_list_mutex);
+   INIT_LIST_HEAD(&instance->bulk_waiter_list);
 
-   default:
-   vchiq_log_error(vchiq_arm_log_level,
-   "Unknown minor device: %d", dev);
-   return -ENXIO;
-   }
+   file->private_data = instance;
 
return 0;
 }
 
-/
-*
-*   vchiq_release
-*
-***/
-
-static int
-vchiq_release(struct inode *inode, struct file *file)
+static int vchiq_release(struct inode *inode, struct file *file)
 {
-   int dev = iminor(inode) & 0x0f;
+   VCHIQ_INSTANCE_T instance = file->private_data;
+   VCHIQ_STATE_T *state = vchiq_get_state();
+   VCHIQ_SERVICE_T *service;
int ret = 0;
+   int i;
 
-   switch (dev) {
-   case VCHIQ_MINOR: {
-   VCHIQ_INSTANCE_T instance = file->private_data;
-   VCHIQ_STATE_T *state = vchiq_get_state();
-   VCHIQ_SERVICE_T *service;
-   int i;
+   vchiq_log_info(vchiq_arm_log_level, "%s: instance=%lx", __func__,
+  (unsigned long)instance);
 
-   vchiq_log_info(vchiq_arm_log_level,
-   "%s: instance=%lx",
-   __func__, (unsigned long)instance);
+   if (!state) {
+   ret = -EPERM;
+   goto out;
+   }
 
-   if (!state) {
-   ret = -EPERM;
-   goto out;
-   }
+   /* Ensure videocore is awake to allow termination. */
+   vchiq_use_internal(instance->state, NULL, USE_TYPE_VCHIQ);
 
-   /* Ensure videocore is awake to allow termination. */
-   vchiq_use_internal(instance->state, NULL,
-   USE_TYPE_VCHIQ);
+   mutex_lock(&i

[PATCH v2 0/5] Remove platform data and introduce DT bindings

[Shreeya Patel]

This patchset introduces device tree bindings for adt7316
driver and removes the usage of platform data from it.
Also, it sets the data field to it's appropriate value in
the i2c read function which was nowhere being set before.

Changes in v2:
  - Make the commit message of patch *[1/5] appropriate.

Shreeya Patel (5):
  Staging: iio: adt7316: Add of_device_id table
  Staging: iio: adt7316: Use device tree data to set ldac_pin
  Staging: iio: adt7316: Switch irq_flags to a local variable
  Staging: iio: adt7316: Change the name from irq_flags to irq_type
  Staging: iio: adt7316: Use device tree data to assign irq_type

 drivers/staging/iio/addac/adt7316-i2c.c | 14 -
 drivers/staging/iio/addac/adt7316-spi.c |  1 -
 drivers/staging/iio/addac/adt7316.c | 39 ++---
 drivers/staging/iio/addac/adt7316.h |  1 -
 4 files changed, 42 insertions(+), 13 deletions(-)

-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/5] Staging: iio: adt7316: Add of_device_id table

[Shreeya Patel]

When the kernel starts up, it kicks off compiled-in drivers
that match “compatible” entries it finds in the device tree.
At a later stage (when /lib/modules is available), all kernel modules
that match “compatible” entries in the device tree are loaded.

But if there is no dt table then there should be a fall back path
with which desired kernel modules can be loaded. Hence, add
of_device_id table in the i2c driver to be able to use when there
is no dt table.

Signed-off-by: Shreeya Patel 
---

Changes in v2:
  - Make the commit message appropriate and assign of_match_table
in the driver structure.

 drivers/staging/iio/addac/adt7316-i2c.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/staging/iio/addac/adt7316-i2c.c 
b/drivers/staging/iio/addac/adt7316-i2c.c
index 473e5e34ec00..41bc4ca008bc 100644
--- a/drivers/staging/iio/addac/adt7316-i2c.c
+++ b/drivers/staging/iio/addac/adt7316-i2c.c
@@ -126,9 +126,22 @@ static const struct i2c_device_id adt7316_i2c_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, adt7316_i2c_id);
 
+static const struct of_device_id adt7316_of_match[] = {
+   { .compatible = "adi,adt7316" },
+   { .compatible = "adi,adt7317" },
+   { .compatible = "adi,adt7318" },
+   { .compatible = "adi,adt7516" },
+   { .compatible = "adi,adt7517" },
+   { .compatible = "adi,adt7519" },
+   { },
+};
+
+MODULE_DEVICE_TABLE(of, adt7316_of_match);
+
 static struct i2c_driver adt7316_driver = {
.driver = {
.name = "adt7316",
+   .of_match_table = adt7316_of_match,
.pm = ADT7316_PM_OPS,
},
.probe = adt7316_i2c_probe,
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/5] Staging: iio: adt7316: Use device tree data to set ldac_pin

[Shreeya Patel]

Make the driver use device tree instead of the platform data.
Hence, use devm_gpiod_get_optional function to get the data from
device tree for ldac-pin and accordingly make the needed changes
in the driver.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/iio/addac/adt7316.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316.c 
b/drivers/staging/iio/addac/adt7316.c
index 3f22d1088713..deb2f7b40f60 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -177,7 +177,7 @@
 
 struct adt7316_chip_info {
struct adt7316_bus  bus;
-   u16 ldac_pin;
+   struct gpio_desc*ldac_pin;
u16 int_mask;   /* 0x2f */
u8  config1;
u8  config2;
@@ -950,8 +950,8 @@ static ssize_t adt7316_store_update_DAC(struct device *dev,
if (ret)
return -EIO;
} else {
-   gpio_set_value(chip->ldac_pin, 0);
-   gpio_set_value(chip->ldac_pin, 1);
+   gpiod_set_value(chip->ldac_pin, 0);
+   gpiod_set_value(chip->ldac_pin, 1);
}
 
return len;
@@ -2120,7 +2120,13 @@ int adt7316_probe(struct device *dev, struct adt7316_bus 
*bus,
else
return -ENODEV;
 
-   chip->ldac_pin = adt7316_platform_data[1];
+   chip->ldac_pin = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_LOW);
+   if (IS_ERR(chip->ldac_pin)) {
+   ret = PTR_ERR(chip->ldac_pin);
+   dev_err(dev, "Failed to request ldac GPIO: %d\n", ret);
+   return ret;
+   }
+
if (chip->ldac_pin) {
chip->config3 |= ADT7316_DA_EN_VIA_DAC_LDCA;
if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/5] Staging: iio: adt7316: Switch irq_flags to a local variable

[Shreeya Patel]

There is no need to store irq_flags into the structure as it
is always set to the same thing. Hence switch irq_flags to a
local variable.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/iio/addac/adt7316-i2c.c | 1 -
 drivers/staging/iio/addac/adt7316-spi.c | 1 -
 drivers/staging/iio/addac/adt7316.c | 8 
 drivers/staging/iio/addac/adt7316.h | 1 -
 4 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316-i2c.c 
b/drivers/staging/iio/addac/adt7316-i2c.c
index 41bc4ca008bc..ac91163656b5 100644
--- a/drivers/staging/iio/addac/adt7316-i2c.c
+++ b/drivers/staging/iio/addac/adt7316-i2c.c
@@ -104,7 +104,6 @@ static int adt7316_i2c_probe(struct i2c_client *client,
struct adt7316_bus bus = {
.client = client,
.irq = client->irq,
-   .irq_flags = IRQF_TRIGGER_LOW,
.read = adt7316_i2c_read,
.write = adt7316_i2c_write,
.multi_read = adt7316_i2c_multi_read,
diff --git a/drivers/staging/iio/addac/adt7316-spi.c 
b/drivers/staging/iio/addac/adt7316-spi.c
index 5cd22743e140..e75827e326a6 100644
--- a/drivers/staging/iio/addac/adt7316-spi.c
+++ b/drivers/staging/iio/addac/adt7316-spi.c
@@ -94,7 +94,6 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
struct adt7316_bus bus = {
.client = spi_dev,
.irq = spi_dev->irq,
-   .irq_flags = IRQF_TRIGGER_LOW,
.read = adt7316_spi_read,
.write = adt7316_spi_write,
.multi_read = adt7316_spi_multi_read,
diff --git a/drivers/staging/iio/addac/adt7316.c 
b/drivers/staging/iio/addac/adt7316.c
index deb2f7b40f60..dfae22619287 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -2102,6 +2102,7 @@ int adt7316_probe(struct device *dev, struct adt7316_bus 
*bus,
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
unsigned short *adt7316_platform_data = dev->platform_data;
+   int irq_flags = IRQF_TRIGGER_LOW;
int ret = 0;
 
indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
@@ -2146,19 +2147,18 @@ int adt7316_probe(struct device *dev, struct 
adt7316_bus *bus,
 
if (chip->bus.irq > 0) {
if (adt7316_platform_data[0])
-   chip->bus.irq_flags = adt7316_platform_data[0];
+   irq_flags = adt7316_platform_data[0];
 
ret = devm_request_threaded_irq(dev, chip->bus.irq,
NULL,
adt7316_event_handler,
-   chip->bus.irq_flags |
-   IRQF_ONESHOT,
+   irq_flags | IRQF_ONESHOT,
indio_dev->name,
indio_dev);
if (ret)
return ret;
 
-   if (chip->bus.irq_flags & IRQF_TRIGGER_HIGH)
+   if (irq_flags & IRQF_TRIGGER_HIGH)
chip->config1 |= ADT7316_INT_POLARITY;
}
 
diff --git a/drivers/staging/iio/addac/adt7316.h 
b/drivers/staging/iio/addac/adt7316.h
index ec40fbb698a6..fd7c5c92b599 100644
--- a/drivers/staging/iio/addac/adt7316.h
+++ b/drivers/staging/iio/addac/adt7316.h
@@ -17,7 +17,6 @@
 struct adt7316_bus {
void *client;
int irq;
-   int irq_flags;
int (*read)(void *client, u8 reg, u8 *data);
int (*write)(void *client, u8 reg, u8 val);
int (*multi_read)(void *client, u8 first_reg, u8 count, u8 *data);
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 4/5] Staging: iio: adt7316: Change the name from irq_flags to irq_type

[Shreeya Patel]

Most of the drivers in IIO uses irq_type as the name for
storing the interrupt type and hence change the name from
irq_flags to irq_type for maintaining the consistency.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/iio/addac/adt7316.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316.c 
b/drivers/staging/iio/addac/adt7316.c
index dfae22619287..9c72538baf9e 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -2102,7 +2102,7 @@ int adt7316_probe(struct device *dev, struct adt7316_bus 
*bus,
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
unsigned short *adt7316_platform_data = dev->platform_data;
-   int irq_flags = IRQF_TRIGGER_LOW;
+   int irq_type = IRQF_TRIGGER_LOW;
int ret = 0;
 
indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
@@ -2147,18 +2147,18 @@ int adt7316_probe(struct device *dev, struct 
adt7316_bus *bus,
 
if (chip->bus.irq > 0) {
if (adt7316_platform_data[0])
-   irq_flags = adt7316_platform_data[0];
+   irq_type = adt7316_platform_data[0];
 
ret = devm_request_threaded_irq(dev, chip->bus.irq,
NULL,
adt7316_event_handler,
-   irq_flags | IRQF_ONESHOT,
+   irq_type | IRQF_ONESHOT,
indio_dev->name,
indio_dev);
if (ret)
return ret;
 
-   if (irq_flags & IRQF_TRIGGER_HIGH)
+   if (irq_type & IRQF_TRIGGER_HIGH)
chip->config1 |= ADT7316_INT_POLARITY;
}
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 5/5] Staging: iio: adt7316: Use device tree data to assign irq_type

[Shreeya Patel]

ADT7316 driver no more uses platform data and hence use device tree
data instead of platform data for assigning irq_type field.
Switch case figures out the type of irq and if it's the default case
then assign the default value to the irq_type i.e.
irq_type = IRQF_TRIGGER_LOW

Signed-off-by: Shreeya Patel 
---
 drivers/staging/iio/addac/adt7316.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316.c 
b/drivers/staging/iio/addac/adt7316.c
index 9c72538baf9e..c647875a64f5 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -2101,8 +2101,7 @@ int adt7316_probe(struct device *dev, struct adt7316_bus 
*bus,
 {
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
-   unsigned short *adt7316_platform_data = dev->platform_data;
-   int irq_type = IRQF_TRIGGER_LOW;
+   int irq_type;
int ret = 0;
 
indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
@@ -2146,8 +2145,22 @@ int adt7316_probe(struct device *dev, struct adt7316_bus 
*bus,
indio_dev->modes = INDIO_DIRECT_MODE;
 
if (chip->bus.irq > 0) {
-   if (adt7316_platform_data[0])
-   irq_type = adt7316_platform_data[0];
+   irq_type =
+   irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
+
+   switch (irq_type) {
+   case IRQF_TRIGGER_HIGH:
+   case IRQF_TRIGGER_RISING:
+   break;
+   case IRQF_TRIGGER_LOW:
+   case IRQF_TRIGGER_FALLING:
+   break;
+   default:
+   dev_info(dev, "mode %d unsupported, using 
IRQF_TRIGGER_LOW\n",
+irq_type);
+   irq_type = IRQF_TRIGGER_LOW;
+   break;
+   }
 
ret = devm_request_threaded_irq(dev, chip->bus.irq,
NULL,
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC PATCH v2] android: ion: How to properly clean caches for uncached allocations

[Brian Starkey]

Hi Liam,

I'm missing a bit of context here, but I did read the v1 thread.
Please accept my apologies if I'm re-treading trodden ground.

I do know we're chasing nebulous ion "problems" on our end, which
certainly seem to be related to what you're trying to fix here.

On Thu, Nov 01, 2018 at 03:15:06PM -0700, Liam Mark wrote:
>Based on the suggestions from Laura I created a first draft for a change
>which will attempt to ensure that uncached mappings are only applied to
>ION memory who's cache lines have been cleaned.
>It does this by providing cached mappings (for uncached ION allocations)
>until the ION buffer is dma mapped and successfully cleaned, then it drops
>the userspace mappings and when pages are accessed they are faulted back
>in and uncached mappings are created.

If I understand right, there's no way to portably clean the cache of
the kernel mapping before we map the pages into userspace. Is that
right?

Alternatively, can we just make ion refuse to give userspace a
non-cached mapping for pages which are mapped in the kernel as cached?
Would userspace using the dma-buf sync ioctl around its accesses do
the "right thing" in that case?

Given that as you pointed out, the kernel does still have a cached
mapping to these pages, trying to give the CPU a non-cached mapping of
those same pages while preserving consistency seems fraught. Wouldn't
it be better to make sure all CPU mappings are cached, and have CPU
clients use the dma_buf_{begin,end}_cpu_access() hooks to get
consistency where needed?

>
>This change has the following potential disadvantages:
>- It assumes that userpace clients won't attempt to access the buffer
>while it is being mapped as we are removing the userpspace mappings at
>this point (though it is okay for them to have it mapped)
>- It assumes that kernel clients won't hold a kernel mapping to the buffer
>(ie dma_buf_kmap) while it is being dma-mapped. What should we do if there
>is a kernel mapping at the time of dma mapping, fail the mapping, warn?
>- There may be a performance penalty as a result of having to fault in the
>pages after removing the userspace mappings.

I wonder if the dma-buf sync ioctl might provide a way for userspace
to opt-in to when the zap/fault happens. Zap on (DMA_BUF_SYNC_WRITE |
DMA_BUF_SYNC_WRITE_END) and fault on (DMA_BUF_SYNC_READ |
DMA_BUF_SYNC_START)

>
>It passes basic testing involving reading writing and reading from
>uncached system heap allocations before and after dma mapping.
>
>Please let me know if this is heading in the right direction and if there
>are any concerns.
>
>Signed-off-by: Liam Mark 
>---
> drivers/staging/android/ion/ion.c | 146 +-
> drivers/staging/android/ion/ion.h |   9 +++
> 2 files changed, 152 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/staging/android/ion/ion.c 
>b/drivers/staging/android/ion/ion.c
>index 99073325b0c0..3dc0f5a265bf 100644
>--- a/drivers/staging/android/ion/ion.c
>+++ b/drivers/staging/android/ion/ion.c
>@@ -96,6 +96,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap 
>*heap,
>   }
>
>   INIT_LIST_HEAD(&buffer->attachments);
>+  INIT_LIST_HEAD(&buffer->vmas);
>   mutex_init(&buffer->lock);
>   mutex_lock(&dev->buffer_lock);
>   ion_buffer_add(dev, buffer);
>@@ -117,6 +118,7 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
>   buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
>   }
>   buffer->heap->ops->free(buffer);
>+  vfree(buffer->pages);
>   kfree(buffer);
> }
>
>@@ -245,11 +247,29 @@ static void ion_dma_buf_detatch(struct dma_buf *dmabuf,
>   kfree(a);
> }
>
>+static bool ion_buffer_uncached_clean(struct ion_buffer *buffer)
>+{
>+  return buffer->uncached_clean;
>+}

nit: The function name sounds like a verb to me - as in "calling this
will clean the buffer". I feel ion_buffer_is_uncached_clean() would
read better.

Thanks,
-Brian

>+
>+/* expect buffer->lock to be already taken */
>+static void ion_buffer_zap_mappings(struct ion_buffer *buffer)
>+{
>+  struct ion_vma_list *vma_list;
>+
>+  list_for_each_entry(vma_list, &buffer->vmas, list) {
>+  struct vm_area_struct *vma = vma_list->vma;
>+
>+  zap_page_range(vma, vma->vm_start, vma->vm_end - vma->vm_start);
>+  }
>+}
>+
> static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
>   enum dma_data_direction direction)
> {
>   struct ion_dma_buf_attachment *a = attachment->priv;
>   struct sg_table *table;
>+  struct ion_buffer *buffer = attachment->dmabuf->priv;
>
>   table = a->table;
>
>@@ -257,6 +277,19 @@ static struct sg_table *ion_map_dma_buf(struct 
>dma_buf_attachment *attachment,
>   direction))
>   return ERR_PTR(-ENOMEM);
>
>+  if (!ion_buffer_cached(buffer)) {
>+  mutex_lock(&buffer->lock);
>+  if (!ion_buffer_uncached_clean(buffer))

Re: Patch "x86/hyper-v: Enable PIT shutdown quirk" has been added to the 4.19-stable tree

[kbuild test robot]

Hi gregkh,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/x86/core]
[cannot apply to v4.20-rc3 next-20181120]
[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/gregkh-linuxfoundation-org/Patch-x86-hyper-v-Enable-PIT-shutdown-quirk-has-been-added-to-the-4-19-stable-tree/20181120-042611
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

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

>> arch/x86/kernel/cpu/mshyperv.c:297:9: error: undefined identifier 
>> 'i8253_clear_counter_on_shutdown'
   arch/x86/kernel/cpu/mshyperv.c:320:41: warning: symbol 'x86_hyper_ms_hyperv' 
was not declared. Should it be static?
>> arch/x86/kernel/cpu/mshyperv.c:297:9: warning: generating address of 
>> non-lvalue (3)
   arch/x86/kernel/cpu/mshyperv.c: In function 'ms_hyperv_init_platform':
   arch/x86/kernel/cpu/mshyperv.c:297:2: error: 
'i8253_clear_counter_on_shutdown' undeclared (first use in this function)
 i8253_clear_counter_on_shutdown = false;
 ^~~
   arch/x86/kernel/cpu/mshyperv.c:297:2: note: each undeclared identifier is 
reported only once for each function it appears in

vim +/i8253_clear_counter_on_shutdown +297 arch/x86/kernel/cpu/mshyperv.c

   275  
   276  #if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE)
   277  machine_ops.shutdown = hv_machine_shutdown;
   278  machine_ops.crash_shutdown = hv_machine_crash_shutdown;
   279  #endif
   280  mark_tsc_unstable("running on Hyper-V");
   281  
   282  /*
   283   * Generation 2 instances don't support reading the NMI status 
from
   284   * 0x61 port.
   285   */
   286  if (efi_enabled(EFI_BOOT))
   287  x86_platform.get_nmi_reason = hv_get_nmi_reason;
   288  
   289  /*
   290   * Hyper-V VMs have a PIT emulation quirk such that zeroing the
   291   * counter register during PIT shutdown restarts the PIT. So it
   292   * continues to interrupt @18.2 HZ. Setting i8253_clear_counter
   293   * to false tells pit_shutdown() not to zero the counter so that
   294   * the PIT really is shutdown. Generation 2 VMs don't have a 
PIT,
   295   * and setting this value has no effect.
   296   */
 > 297  i8253_clear_counter_on_shutdown = false;
   298  

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


.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/2] staging: iio: ad7606: Move out of staging

[Stefan Popa]

Move ad7606 ADC driver out of staging and into the mainline.

Signed-off-by: Stefan Popa 
---
Changes in v2:
- Simplified the Kconfig menu.
- Added SPDX-License-Identifier.
- Ordered the includes alphabetically.
- Used a threaded interrupt.
- Replaced ad7606_poll_bh_to_ring() with ad7606_trigger_handler().
- Used a trigger. 
- Replaced wait_event_interruptible() with 
wait_for_completion_timeout().
- Replaced wake_up_interruptible() with complete().
- Used devm_iio_triggered_buffer_setup().
- Added buffer_ops.
- Used single line comments where needed.
- Removed the gap between docs and struct.
- Added ad7606_of_match[].

 MAINTAINERS  |   7 +
 drivers/iio/adc/Kconfig  |  28 ++
 drivers/iio/adc/Makefile |   3 +
 drivers/iio/adc/ad7606.c | 608 +++
 drivers/iio/adc/ad7606.h | 107 ++
 drivers/iio/adc/ad7606_par.c | 110 +++
 drivers/iio/adc/ad7606_spi.c |  88 +
 drivers/staging/iio/adc/Kconfig  |  34 --
 drivers/staging/iio/adc/Makefile |   3 -
 drivers/staging/iio/adc/ad7606.c | 565 
 drivers/staging/iio/adc/ad7606.h | 106 --
 drivers/staging/iio/adc/ad7606_par.c | 113 ---
 drivers/staging/iio/adc/ad7606_spi.c |  79 -
 13 files changed, 951 insertions(+), 900 deletions(-)
 create mode 100644 drivers/iio/adc/ad7606.c
 create mode 100644 drivers/iio/adc/ad7606.h
 create mode 100644 drivers/iio/adc/ad7606_par.c
 create mode 100644 drivers/iio/adc/ad7606_spi.c
 delete mode 100644 drivers/staging/iio/adc/ad7606.c
 delete mode 100644 drivers/staging/iio/adc/ad7606.h
 delete mode 100644 drivers/staging/iio/adc/ad7606_par.c
 delete mode 100644 drivers/staging/iio/adc/ad7606_spi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f642044..843545d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -839,6 +839,13 @@ S: Supported
 F: drivers/iio/dac/ad5758.c
 F: Documentation/devicetree/bindings/iio/dac/ad5758.txt
 
+ANALOG DEVICES INC AD7606 DRIVER
+M: Stefan Popa 
+L: linux-...@vger.kernel.org
+W: http://ez.analog.com/community/linux-device-drivers
+S: Supported
+F: drivers/iio/adc/ad7606.c
+
 ANALOG DEVICES INC AD9389B DRIVER
 M: Hans Verkuil 
 L: linux-me...@vger.kernel.org
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a52fea8..c3f61c9 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -58,6 +58,34 @@ config AD7476
  To compile this driver as a module, choose M here: the
  module will be called ad7476.
 
+config AD7606
+   tristate
+   depends on GPIOLIB || COMPILE_TEST
+   depends on HAS_IOMEM
+   select IIO_BUFFER
+   select IIO_TRIGGERED_BUFFER
+
+config AD7606_IFACE_PARALLEL
+   tristate "Analog Devices AD7606 ADC driver with parallel interface 
support"
+   select AD7606
+   help
+ Say yes here to build parallel interface support for Analog Devices:
+ ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters 
(ADC).
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad7606_parallel.
+
+config AD7606_IFACE_SPI
+   tristate "Analog Devices AD7606 ADC driver with spi interface support"
+   depends on SPI
+   select AD7606
+   help
+ Say yes here to build spi interface support for Analog Devices:
+ ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters 
(ADC).
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad7606_spi.
+
 config AD7766
tristate "Analog Devices AD7766/AD7767 ADC driver"
depends on SPI_MASTER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index a6e6a0b..b734f4f 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -8,6 +8,9 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD7266) += ad7266.o
 obj-$(CONFIG_AD7291) += ad7291.o
 obj-$(CONFIG_AD7298) += ad7298.o
+obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
+obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
+obj-$(CONFIG_AD7606) += ad7606.o
 obj-$(CONFIG_AD7923) += ad7923.o
 obj-$(CONFIG_AD7476) += ad7476.o
 obj-$(CONFIG_AD7766) += ad7766.o
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
new file mode 100644
index 000..4e09635
--- /dev/null
+++ b/drivers/iio/adc/ad7606.c
@@ -0,0 +1,608 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD7606 SPI ADC driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ad7606.h"
+
+/*
+ * Scales are computed as 5000/32768 and 1/32768 respectively,
+ * so that

Re: [PATCH v1 8/8] PM / Hibernate: exclude all PageOffline() pages

[Rafael J. Wysocki]

On Monday, November 19, 2018 11:16:16 AM CET David Hildenbrand wrote:
> The content of pages that are marked PG_offline is not of interest
> (e.g. inflated by a balloon driver), let's skip these pages.
> 
> Cc: "Rafael J. Wysocki" 
> Cc: Pavel Machek 
> Cc: Len Brown 
> Cc: Andrew Morton 
> Cc: Matthew Wilcox 
> Cc: Michal Hocko 
> Cc: "Michael S. Tsirkin" 
> Acked-by: Pavel Machek 
> Signed-off-by: David Hildenbrand 

Acked-by: Rafael J. Wysocki 

> ---
>  kernel/power/snapshot.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 87e6dd57819f..8d7b4d458842 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1222,7 +1222,7 @@ static struct page *saveable_highmem_page(struct zone 
> *zone, unsigned long pfn)
>   BUG_ON(!PageHighMem(page));
>  
>   if (swsusp_page_is_forbidden(page) ||  swsusp_page_is_free(page) ||
> - PageReserved(page))
> + PageReserved(page) || PageOffline(page))
>   return NULL;
>  
>   if (page_is_guard(page))
> @@ -1286,6 +1286,9 @@ static struct page *saveable_page(struct zone *zone, 
> unsigned long pfn)
>   if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
>   return NULL;
>  
> + if (PageOffline(page))
> + return NULL;
> +
>   if (PageReserved(page)
>   && (!kernel_page_present(page) || pfn_is_nosave(pfn)))
>   return NULL;
> 


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 7/8] PM / Hibernate: use pfn_to_online_page()

[Rafael J. Wysocki]

On Monday, November 19, 2018 11:16:15 AM CET David Hildenbrand wrote:
> Let's use pfn_to_online_page() instead of pfn_to_page() when checking
> for saveable pages to not save/restore offline memory sections.
> 
> Cc: "Rafael J. Wysocki" 
> Cc: Pavel Machek 
> Cc: Len Brown 
> Cc: Andrew Morton 
> Cc: Matthew Wilcox 
> Cc: Michal Hocko 
> Cc: "Michael S. Tsirkin" 
> Suggested-by: Michal Hocko 
> Signed-off-by: David Hildenbrand 

Acked-by: Rafael J. Wysocki 

> ---
>  kernel/power/snapshot.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 640b2034edd6..87e6dd57819f 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1215,8 +1215,8 @@ static struct page *saveable_highmem_page(struct zone 
> *zone, unsigned long pfn)
>   if (!pfn_valid(pfn))
>   return NULL;
>  
> - page = pfn_to_page(pfn);
> - if (page_zone(page) != zone)
> + page = pfn_to_online_page(pfn);
> + if (!page || page_zone(page) != zone)
>   return NULL;
>  
>   BUG_ON(!PageHighMem(page));
> @@ -1277,8 +1277,8 @@ static struct page *saveable_page(struct zone *zone, 
> unsigned long pfn)
>   if (!pfn_valid(pfn))
>   return NULL;
>  
> - page = pfn_to_page(pfn);
> - if (page_zone(page) != zone)
> + page = pfn_to_online_page(pfn);
> + if (!page || page_zone(page) != zone)
>   return NULL;
>  
>   BUG_ON(PageHighMem(page));
> 


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH net-next 2/4] net/hyperv: use skb_vlan_tag_*() helpers

[Haiyang Zhang]

> -Original Message-
> From: Michał Mirosław 
> Sent: Tuesday, November 20, 2018 7:21 AM
> To: net...@vger.kernel.org
> Cc: KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger
> ; de...@linuxdriverproject.org; Ajit Khaparde
> ; Leon Romanovsky ;
> linux-r...@vger.kernel.org; Saeed Mahameed ;
> Sathya Perla ; Somnath Kotur
> ; Sriharsha Basavapatna
> 
> Subject: [PATCH net-next 2/4] net/hyperv: use skb_vlan_tag_*() helpers
> 
> Replace open-coded bitfield manipulation with skb_vlan_tag_*() helpers.
> This also enables correctly passing of VLAN.CFI bit.
> 
> Signed-off-by: Michał Mirosław 
> ---

Reviewed-by: Haiyang Zhang 
Thanks.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 4/7] dt-bindings:iio:resolver: Add docs for ad2s90

[Matheus Tavares Bernardino]

On Mon, Nov 19, 2018 at 6:22 AM Ardelean, Alexandru
 wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds the device tree binding documentation for the ad2s90
> > resolver-to-digital converter.
> >
>
> One minor comment inline.
>
> > Signed-off-by: Matheus Tavares 
> > ---
> > Changes in v2:
> >  - Rewritten 'spi-cpol and spi-cpha' item to say that the device can
> >  work in either mode (0,0) or (1,1) and explain how they should be
> >  specified in DT.
> >
> >  .../bindings/iio/resolver/ad2s90.txt  | 28 +++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > new file mode 100644
> > index ..594417539938
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > @@ -0,0 +1,28 @@
> > +Analog Devices AD2S90 Resolver-to-Digital Converter
> > +
> > +https://www.analog.com/en/products/ad2s90.html
> > +
> > +Required properties:
> > +  - compatible: should be "adi,ad2s90"
> > +  - reg: SPI chip select number for the device
> > +  - spi-max-frequency: set maximum clock frequency, must be 83
> > +  - spi-cpol and spi-cpha:
> > +Either SPI mode (0,0) or (1,1) must be used, so specify none or
> > both of
> > +spi-cpha, spi-cpol.
> For SPI properties it's a good idea to also reference the document for SPI
> bindings.
> Something like:
> See for more details:
>  Documentation/devicetree/bindings/spi/spi-bus.txt
>

Thanks, Alex! I'll add that for v3.

Also, can you confirm AD2S90 works in both spi mode 0 and 3? It's not
explicitly stated in the datasheet, but that's what it seemed to me
and some colleagues.

Thanks,
Matheus

> > +
> > +Note about max frequency:
> > +Chip's max frequency, as specified in its datasheet, is 2Mhz. But a
> > 600ns
> > +delay is expected between the application of a logic LO to CS and
> > the
> > +application of SCLK, as also specified. And since the delay is not
> > +implemented in the spi code, to satisfy it, SCLK's period should be
> > at most
> > +2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
> > gives
> > +roughly 83Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <83>;
> > + spi-cpol;
> > + spi-cpha;
> > +};
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/563614e00314ba92b9513645a82fde06504a42d5.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/7] staging:iio:ad2s90: Add device tree support

[Matheus Tavares Bernardino]

On Mon, Nov 19, 2018 at 6:09 AM Ardelean, Alexandru
 wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds device tree support to ad2s90 with standard
> > device tree id table.
> >
>
> Hey,
>
> Comment inline
>
> > Signed-off-by: Matheus Tavares 
> > ---
> > Changes in v2:
> >  - none
> >
> >  drivers/staging/iio/resolver/ad2s90.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index 3e257ac46f7a..6ffbac66b837 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
> >   return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> >  }
> >
> > +static const struct of_device_id ad2s90_of_match[] = {
> > + { .compatible = "adi,ad2s90", },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ad2s90_of_match);
> > +
> >  static const struct spi_device_id ad2s90_id[] = {
> >   { "ad2s90" },
> >   {}
> > @@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
> >  static struct spi_driver ad2s90_driver = {
> >   .driver = {
> >   .name = "ad2s90",
> > + .of_match_table = of_match_ptr(ad2s90_of_match),
>
> I think you need to remove the of_match_ptr().
> There was a comment from Jonathan on another thread about this.
> See:
>https://patchwork.kernel.org/patch/10682963/
>

Hm, got it, thanks!

I don't understand much about ACPI yet, and I had understood the
"of_match_ptr" as a guard. Could someone point me in which cases it
should be used? Or is it obsolete?

Matheus

> So,
> +   .of_match_table = of_match_ptr(ad2s90_of_match),
>
> becomes
> > + .of_match_table = ad2s90_of_match,
>
> >   },
> >   .probe = ad2s90_probe,
> >   .id_table = ad2s90_id,
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/f250fa3a01b51d59979e7a2e3e42cc34d02aa52e.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[Pankaj Gupta]

> > >> ---
> > >>  drivers/hv/hv_balloon.c | 14 --
> > >>  1 file changed, 12 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> > >> index 211f3fe3a038..47719862e57f 100644
> > >> --- a/drivers/hv/hv_balloon.c
> > >> +++ b/drivers/hv/hv_balloon.c
> > >> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
> > >>  /* Check if the particular page is backed and can be onlined and online
> > >>  it.
> > >>  */
> > >>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page
> > >>  *pg)
> > >>  {
> > >> -if (!has_pfn_is_backed(has, page_to_pfn(pg)))
> > >> +if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
> > >> +if (!PageOffline(pg))
> > >> +__SetPageOffline(pg);
> > >>  return;
> > >> +}
> > >> +if (PageOffline(pg))
> > >> +__ClearPageOffline(pg);
> > >>  
> > >>  /* This frame is currently backed; online the page. */
> > >>  __online_page_set_limits(pg);
> > >> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct
> > >> hv_dynmem_device
> > >> *dm,
> > >>  
> > >>  for (i = 0; i < num_pages; i++) {
> > >>  pg = pfn_to_page(i + start_frame);
> > >> +__ClearPageOffline(pg);
> > > 
> > > Just thinking, do we need to care for clearing PageOffline flag before
> > > freeing
> > > a balloon'd page?
> > 
> > Yes we have to otherwise the code will crash when trying to set PageBuddy.
> > 
> > (only one page type at a time may be set right now, and it makes sense.
> > A page that is offline cannot e.g. be a buddy page)
> 
> o.k
> > 
> > So PageOffline is completely managed by the page owner.
> 
> Makes sense. Thanks for explaining.

Looks good to me.

Acked-by: Pankaj gupta 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 1/8] mm: balloon: update comment about isolation/migration/compaction

[Michael S. Tsirkin]

On Mon, Nov 19, 2018 at 11:16:09AM +0100, David Hildenbrand wrote:
> Commit b1123ea6d3b3 ("mm: balloon: use general non-lru movable page
> feature") reworked balloon handling to make use of the general
> non-lru movable page feature. The big comment block in
> balloon_compaction.h contains quite some outdated information. Let's fix
> this.
> 
> Cc: Andrew Morton 
> Cc: Matthew Wilcox 
> Cc: Michal Hocko 
> Cc: "Michael S. Tsirkin" 
> Signed-off-by: David Hildenbrand 

Acked-by: Michael S. Tsirkin 

> ---
>  include/linux/balloon_compaction.h | 26 +-
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/balloon_compaction.h 
> b/include/linux/balloon_compaction.h
> index 53051f3d8f25..cbe50da5a59d 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -4,15 +4,18 @@
>   *
>   * Common interface definitions for making balloon pages movable by 
> compaction.
>   *
> - * Despite being perfectly possible to perform ballooned pages migration, 
> they
> - * make a special corner case to compaction scans because balloon pages are 
> not
> - * enlisted at any LRU list like the other pages we do compact / migrate.
> + * Balloon page migration makes use of the general non-lru movable page
> + * feature.
> + *
> + * page->private is used to reference the responsible balloon device.
> + * page->mapping is used in context of non-lru page migration to reference
> + * the address space operations for page isolation/migration/compaction.
>   *
>   * As the page isolation scanning step a compaction thread does is a lockless
>   * procedure (from a page standpoint), it might bring some racy situations 
> while
>   * performing balloon page compaction. In order to sort out these racy 
> scenarios
>   * and safely perform balloon's page compaction and migration we must, 
> always,
> - * ensure following these three simple rules:
> + * ensure following these simple rules:
>   *
>   *   i. when updating a balloon's page ->mapping element, strictly do it 
> under
>   *  the following lock order, independently of the far superior
> @@ -21,19 +24,8 @@
>   * +--spin_lock_irq(&b_dev_info->pages_lock);
>   *   ... page->mapping updates here ...
>   *
> - *  ii. before isolating or dequeueing a balloon page from the balloon device
> - *  pages list, the page reference counter must be raised by one and the
> - *  extra refcount must be dropped when the page is enqueued back into
> - *  the balloon device page list, thus a balloon page keeps its reference
> - *  counter raised only while it is under our special handling;
> - *
> - * iii. after the lockless scan step have selected a potential balloon page 
> for
> - *  isolation, re-test the PageBalloon mark and the PagePrivate flag
> - *  under the proper page lock, to ensure isolating a valid balloon page
> - *  (not yet isolated, nor under release procedure)
> - *
> - *  iv. isolation or dequeueing procedure must clear PagePrivate flag under
> - *  page lock together with removing page from balloon device page list.
> + *  ii. isolation or dequeueing procedure must remove the page from balloon
> + *  device page list under b_dev_info->pages_lock.
>   *
>   * The functions provided by this interface are placed to help on coping with
>   * the aforementioned balloon page corner case, as well as to ensure the 
> simple
> -- 
> 2.17.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 2/8] mm: convert PG_balloon to PG_offline

[Michael S. Tsirkin]

On Mon, Nov 19, 2018 at 11:16:10AM +0100, David Hildenbrand wrote:
> PG_balloon was introduced to implement page migration/compaction for pages
> inflated in virtio-balloon. Nowadays, it is only a marker that a page is
> part of virtio-balloon and therefore logically offline.
> 
> We also want to make use of this flag in other balloon drivers - for
> inflated pages or when onlining a section but keeping some pages offline
> (e.g. used right now by XEN and Hyper-V via set_online_page_callback()).
> 
> We are going to expose this flag to dump tools like makedumpfile. But
> instead of exposing PG_balloon, let's generalize the concept of marking
> pages as logically offline, so it can be reused for other purposes
> later on.
> 
> Rename PG_balloon to PG_offline. This is an indicator that the page is
> logically offline, the content stale and that it should not be touched
> (e.g. a hypervisor would have to allocate backing storage in order for the
> guest to dump an unused page).  We can then e.g. exclude such pages from
> dumps.
> 
> We replace and reuse KPF_BALLOON (23), as this shouldn't really harm
> (and for now the semantics stay the same).  In following patches, we will
> make use of this bit also in other balloon drivers. While at it, document
> PGTABLE.
> 
> Cc: Jonathan Corbet 
> Cc: Alexey Dobriyan 
> Cc: Mike Rapoport 
> Cc: Andrew Morton 
> Cc: Christian Hansen 
> Cc: Vlastimil Babka 
> Cc: "Kirill A. Shutemov" 
> Cc: Stephen Rothwell 
> Cc: Matthew Wilcox 
> Cc: "Michael S. Tsirkin" 
> Cc: Michal Hocko 
> Cc: Pavel Tatashin 
> Cc: Alexander Duyck 
> Cc: Naoya Horiguchi 
> Cc: Miles Chen 
> Cc: David Rientjes 
> Cc: Konstantin Khlebnikov 
> Cc: Kazuhito Hagio 
> Signed-off-by: David Hildenbrand 
> ---
>  Documentation/admin-guide/mm/pagemap.rst |  9 ++---
>  fs/proc/page.c   |  4 ++--
>  include/linux/balloon_compaction.h   |  8 
>  include/linux/page-flags.h   | 11 +++
>  include/uapi/linux/kernel-page-flags.h   |  2 +-
>  tools/vm/page-types.c|  2 +-
>  6 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/admin-guide/mm/pagemap.rst 
> b/Documentation/admin-guide/mm/pagemap.rst
> index 3f7bade2c231..340a5aee9b80 100644
> --- a/Documentation/admin-guide/mm/pagemap.rst
> +++ b/Documentation/admin-guide/mm/pagemap.rst
> @@ -75,9 +75,10 @@ number of times a page is mapped.
>  20. NOPAGE
>  21. KSM
>  22. THP
> -23. BALLOON
> +23. OFFLINE
>  24. ZERO_PAGE
>  25. IDLE
> +26. PGTABLE
>  
>   * ``/proc/kpagecgroup``.  This file contains a 64-bit inode number of the
> memory cgroup each page is charged to, indexed by PFN. Only available when
> @@ -118,8 +119,8 @@ Short descriptions to the page flags
>  identical memory pages dynamically shared between one or more processes
>  22 - THP
>  contiguous pages which construct transparent hugepages
> -23 - BALLOON
> -balloon compaction page
> +23 - OFFLINE
> +page is logically offline
>  24 - ZERO_PAGE
>  zero page for pfn_zero or huge_zero page
>  25 - IDLE
> @@ -128,6 +129,8 @@ Short descriptions to the page flags
>  Note that this flag may be stale in case the page was accessed via
>  a PTE. To make sure the flag is up-to-date one has to read
>  ``/sys/kernel/mm/page_idle/bitmap`` first.
> +26 - PGTABLE
> +page is in use as a page table
>  
>  IO related page flags
>  -
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 6c517b11acf8..378401af4d9d 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -152,8 +152,8 @@ u64 stable_page_flags(struct page *page)
>   else if (page_count(page) == 0 && is_free_buddy_page(page))
>   u |= 1 << KPF_BUDDY;
>  
> - if (PageBalloon(page))
> - u |= 1 << KPF_BALLOON;
> + if (PageOffline(page))
> + u |= 1 << KPF_OFFLINE;
>   if (PageTable(page))
>   u |= 1 << KPF_PGTABLE;
>  
> diff --git a/include/linux/balloon_compaction.h 
> b/include/linux/balloon_compaction.h
> index cbe50da5a59d..f111c780ef1d 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -95,7 +95,7 @@ extern int balloon_page_migrate(struct address_space 
> *mapping,
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>  struct page *page)
>  {
> - __SetPageBalloon(page);
> + __SetPageOffline(page);
>   __SetPageMovable(page, balloon->inode->i_mapping);
>   set_page_private(page, (unsigned long)balloon);
>   list_add(&page->lru, &balloon->pages);
> @@ -111,7 +111,7 @@ static inline void balloon_page_insert(struct 
> balloon_dev_info *balloon,
>   */
>  static inline void balloon_page_delete(struct page *page)
>  {
> - __ClearPageBalloon(page);
> + __ClearPageOffline(page);
>   __ClearPageMovable(page);
>   set_page_private(page, 0);
>   /*
> @@ -141,13 +

Re: [PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[Michael S. Tsirkin]

On Mon, Nov 19, 2018 at 11:16:11AM +0100, David Hildenbrand wrote:
> Right now, pages inflated as part of a balloon driver will be dumped
> by dump tools like makedumpfile. While XEN is able to check in the
> crash kernel whether a certain pfn is actuall backed by memory in the
> hypervisor (see xen_oldmem_pfn_is_ram) and optimize this case, dumps of
> other balloon inflated memory will essentially result in zero pages getting
> allocated by the hypervisor and the dump getting filled with this data.
> 
> The allocation and reading of zero pages can directly be avoided if a
> dumping tool could know which pages only contain stale information not to
> be dumped.
> 
> We now have PG_offline which can be (and already is by virtio-balloon)
> used for marking pages as logically offline. Follow up patches will
> make use of this flag also in other balloon implementations.
> 
> Let's export PG_offline via PAGE_OFFLINE_MAPCOUNT_VALUE, so
> makedumpfile can directly skip pages that are logically offline and the
> content therefore stale.
> 
> Please note that this is also helpful for a problem we were seeing under
> Hyper-V: Dumping logically offline memory (pages kept fake offline while
> onlining a section via online_page_callback) would under some condicions
> result in a kernel panic when dumping them.
> 
> Cc: Andrew Morton 
> Cc: Dave Young 
> Cc: "Kirill A. Shutemov" 
> Cc: Baoquan He 
> Cc: Omar Sandoval 
> Cc: Arnd Bergmann 
> Cc: Matthew Wilcox 
> Cc: Michal Hocko 
> Cc: "Michael S. Tsirkin" 
> Cc: Lianbo Jiang 
> Cc: Borislav Petkov 
> Cc: Kazuhito Hagio 
> Signed-off-by: David Hildenbrand 

Acked-by: Michael S. Tsirkin 

> ---
>  kernel/crash_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 933cb3e45b98..093c9f917ed0 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
>   VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
>  #ifdef CONFIG_HUGETLB_PAGE
>   VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
> +#define PAGE_OFFLINE_MAPCOUNT_VALUE  (~PG_offline)
> + VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>  #endif
>  
>   arch_crash_save_vmcoreinfo();
> -- 
> 2.17.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 2/8] mm: convert PG_balloon to PG_offline

[Pankaj Gupta]

> 
> PG_balloon was introduced to implement page migration/compaction for pages
> inflated in virtio-balloon. Nowadays, it is only a marker that a page is
> part of virtio-balloon and therefore logically offline.
> 
> We also want to make use of this flag in other balloon drivers - for
> inflated pages or when onlining a section but keeping some pages offline
> (e.g. used right now by XEN and Hyper-V via set_online_page_callback()).
> 
> We are going to expose this flag to dump tools like makedumpfile. But
> instead of exposing PG_balloon, let's generalize the concept of marking
> pages as logically offline, so it can be reused for other purposes
> later on.
> 
> Rename PG_balloon to PG_offline. This is an indicator that the page is
> logically offline, the content stale and that it should not be touched
> (e.g. a hypervisor would have to allocate backing storage in order for the
> guest to dump an unused page).  We can then e.g. exclude such pages from
> dumps.
> 
> We replace and reuse KPF_BALLOON (23), as this shouldn't really harm
> (and for now the semantics stay the same).  In following patches, we will
> make use of this bit also in other balloon drivers. While at it, document
> PGTABLE.
> 
> Cc: Jonathan Corbet 
> Cc: Alexey Dobriyan 
> Cc: Mike Rapoport 
> Cc: Andrew Morton 
> Cc: Christian Hansen 
> Cc: Vlastimil Babka 
> Cc: "Kirill A. Shutemov" 
> Cc: Stephen Rothwell 
> Cc: Matthew Wilcox 
> Cc: "Michael S. Tsirkin" 
> Cc: Michal Hocko 
> Cc: Pavel Tatashin 
> Cc: Alexander Duyck 
> Cc: Naoya Horiguchi 
> Cc: Miles Chen 
> Cc: David Rientjes 
> Cc: Konstantin Khlebnikov 
> Cc: Kazuhito Hagio 
> Signed-off-by: David Hildenbrand 
> ---
>  Documentation/admin-guide/mm/pagemap.rst |  9 ++---
>  fs/proc/page.c   |  4 ++--
>  include/linux/balloon_compaction.h   |  8 
>  include/linux/page-flags.h   | 11 +++
>  include/uapi/linux/kernel-page-flags.h   |  2 +-
>  tools/vm/page-types.c|  2 +-
>  6 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/admin-guide/mm/pagemap.rst
> b/Documentation/admin-guide/mm/pagemap.rst
> index 3f7bade2c231..340a5aee9b80 100644
> --- a/Documentation/admin-guide/mm/pagemap.rst
> +++ b/Documentation/admin-guide/mm/pagemap.rst
> @@ -75,9 +75,10 @@ number of times a page is mapped.
>  20. NOPAGE
>  21. KSM
>  22. THP
> -23. BALLOON
> +23. OFFLINE
>  24. ZERO_PAGE
>  25. IDLE
> +26. PGTABLE
>  
>   * ``/proc/kpagecgroup``.  This file contains a 64-bit inode number of the
> memory cgroup each page is charged to, indexed by PFN. Only available
> when
> @@ -118,8 +119,8 @@ Short descriptions to the page flags
>  identical memory pages dynamically shared between one or more processes
>  22 - THP
>  contiguous pages which construct transparent hugepages
> -23 - BALLOON
> -balloon compaction page
> +23 - OFFLINE
> +page is logically offline
>  24 - ZERO_PAGE
>  zero page for pfn_zero or huge_zero page
>  25 - IDLE
> @@ -128,6 +129,8 @@ Short descriptions to the page flags
>  Note that this flag may be stale in case the page was accessed via
>  a PTE. To make sure the flag is up-to-date one has to read
>  ``/sys/kernel/mm/page_idle/bitmap`` first.
> +26 - PGTABLE
> +page is in use as a page table
>  
>  IO related page flags
>  -
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 6c517b11acf8..378401af4d9d 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -152,8 +152,8 @@ u64 stable_page_flags(struct page *page)
>   else if (page_count(page) == 0 && is_free_buddy_page(page))
>   u |= 1 << KPF_BUDDY;
>  
> - if (PageBalloon(page))
> - u |= 1 << KPF_BALLOON;
> + if (PageOffline(page))
> + u |= 1 << KPF_OFFLINE;
>   if (PageTable(page))
>   u |= 1 << KPF_PGTABLE;
>  
> diff --git a/include/linux/balloon_compaction.h
> b/include/linux/balloon_compaction.h
> index cbe50da5a59d..f111c780ef1d 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -95,7 +95,7 @@ extern int balloon_page_migrate(struct address_space
> *mapping,
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>  struct page *page)
>  {
> - __SetPageBalloon(page);
> + __SetPageOffline(page);
>   __SetPageMovable(page, balloon->inode->i_mapping);
>   set_page_private(page, (unsigned long)balloon);
>   list_add(&page->lru, &balloon->pages);
> @@ -111,7 +111,7 @@ static inline void balloon_page_insert(struct
> balloon_dev_info *balloon,
>   */
>  static inline void balloon_page_delete(struct page *page)
>  {
> - __ClearPageBalloon(page);
> + __ClearPageOffline(page);
>   __ClearPageMovable(page);
>   set_page_private(page, 0);
>   /*
> @@ -141,13 +141,13 @@ static inline gfp_t balloon_mapping_gfp_mask(void)

Re: [PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[Dave Young]

On 11/19/18 at 11:16am, David Hildenbrand wrote:
> Right now, pages inflated as part of a balloon driver will be dumped
> by dump tools like makedumpfile. While XEN is able to check in the
> crash kernel whether a certain pfn is actuall backed by memory in the
> hypervisor (see xen_oldmem_pfn_is_ram) and optimize this case, dumps of
> other balloon inflated memory will essentially result in zero pages getting
> allocated by the hypervisor and the dump getting filled with this data.
> 
> The allocation and reading of zero pages can directly be avoided if a
> dumping tool could know which pages only contain stale information not to
> be dumped.
> 
> We now have PG_offline which can be (and already is by virtio-balloon)
> used for marking pages as logically offline. Follow up patches will
> make use of this flag also in other balloon implementations.
> 
> Let's export PG_offline via PAGE_OFFLINE_MAPCOUNT_VALUE, so
> makedumpfile can directly skip pages that are logically offline and the
> content therefore stale.
> 
> Please note that this is also helpful for a problem we were seeing under
> Hyper-V: Dumping logically offline memory (pages kept fake offline while
> onlining a section via online_page_callback) would under some condicions
> result in a kernel panic when dumping them.
> 
> Cc: Andrew Morton 
> Cc: Dave Young 
> Cc: "Kirill A. Shutemov" 
> Cc: Baoquan He 
> Cc: Omar Sandoval 
> Cc: Arnd Bergmann 
> Cc: Matthew Wilcox 
> Cc: Michal Hocko 
> Cc: "Michael S. Tsirkin" 
> Cc: Lianbo Jiang 
> Cc: Borislav Petkov 
> Cc: Kazuhito Hagio 
> Signed-off-by: David Hildenbrand 
> ---
>  kernel/crash_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 933cb3e45b98..093c9f917ed0 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
>   VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
>  #ifdef CONFIG_HUGETLB_PAGE
>   VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
> +#define PAGE_OFFLINE_MAPCOUNT_VALUE  (~PG_offline)
> + VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>  #endif
>  
>   arch_crash_save_vmcoreinfo();
> -- 
> 2.17.2
> 

Acked-by: Dave Young 

Thanks
Dave
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 6/8] vmw_balloon: mark inflated pages PG_offline

[Nadav Amit]

Thanks for this patch!

> On Nov 19, 2018, at 2:16 AM, David Hildenbrand  wrote:
> 
> Mark inflated and never onlined pages PG_offline, to tell the world that
> the content is stale and should not be dumped.
> 
> Cc: Xavier Deguillard 
> Cc: Nadav Amit 
> Cc: Arnd Bergmann 
> Cc: Greg Kroah-Hartman 
> Cc: Julien Freche 
> Cc: Andrew Morton 
> Cc: Matthew Wilcox 
> Cc: Michal Hocko 
> Cc: "Michael S. Tsirkin" 
> Signed-off-by: David Hildenbrand 
> ---
> drivers/misc/vmw_balloon.c | 32 
> 1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index e6126a4b95d3..8cc8bd9a4e32 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -544,6 +544,36 @@ unsigned int vmballoon_page_order(enum 
> vmballoon_page_size_type page_size)
>   return page_size == VMW_BALLOON_2M_PAGE ? VMW_BALLOON_2M_ORDER : 0;
> }
> 
> +/**
> + * vmballoon_mark_page_offline() - mark a page as offline
> + * @page: pointer for the page

If possible, please add a period at the end of the sentence (yes, I know I
got it wrong in some places too).

> + * @page_size: the size of the page.
> + */
> +static void
> +vmballoon_mark_page_offline(struct page *page,
> + enum vmballoon_page_size_type page_size)
> +{
> + int i;
> +
> + for (i = 0; i < 1ULL << vmballoon_page_order(page_size); i++)

Can you please do instead:

unsigned int;

for (i = 0; i < vmballoon_page_in_frames(page_size); i++)


> + __SetPageOffline(page + i);
> +}
> +
> +/**
> + * vmballoon_mark_page_online() - mark a page as online
> + * @page: pointer for the page
> + * @page_size: the size of the page.
> + */
> +static void
> +vmballoon_mark_page_online(struct page *page,
> +enum vmballoon_page_size_type page_size)
> +{
> + int i;
> +
> + for (i = 0; i < 1ULL << vmballoon_page_order(page_size); i++)
> + __ClearPageOffline(page + i);

Same here (use vmballoon_page_in_frames).

> +}
> +
> /**
>  * vmballoon_page_in_frames() - returns the number of frames in a page.
>  * @page_size: the size of the page.
> @@ -612,6 +642,7 @@ static int vmballoon_alloc_page_list(struct vmballoon *b,
>ctl->page_size);
> 
>   if (page) {
> + vmballoon_mark_page_offline(page, ctl->page_size);
>   /* Success. Add the page to the list and continue. */
>   list_add(&page->lru, &ctl->pages);
>   continue;
> @@ -850,6 +881,7 @@ static void vmballoon_release_page_list(struct list_head 
> *page_list,
> 
>   list_for_each_entry_safe(page, tmp, page_list, lru) {
>   list_del(&page->lru);
> + vmballoon_mark_page_online(page, page_size);
>   __free_pages(page, vmballoon_page_order(page_size));
>   }

We would like to test it in the next few days, but in the meanwhile, after
you address these minor issues:

Acked-by: Nadav Amit 

Thanks again,
Nadav 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[Baoquan He]

On 11/19/18 at 11:16am, David Hildenbrand wrote:
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 933cb3e45b98..093c9f917ed0 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
>   VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
>  #ifdef CONFIG_HUGETLB_PAGE
>   VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
> +#define PAGE_OFFLINE_MAPCOUNT_VALUE  (~PG_offline)
> + VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>  #endif

This solution looks good to me. One small concern is why we don't
export PG_offline to vmcoreinfo directly, then define
PAGE_OFFLINE_MAPCOUNT_VALUE in makedumpfile. We have been exporting
kernel data/MACRO directly, why this one is exceptional.

Thanks
Baoquan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel