* Re: [RFC 0/1] Appletalk AARP probe broken by receipt of own broadcasts.
From: Craig McGeachie @ 2018-08-21 6:07 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David S. Miller, netdev, Craig McGeachie
In-Reply-To: <20180820132831.GA6583@lunn.ch>
On 21/08/18 01:28, Andrew Lunn wrote:
>> Turns out the problem is WinPCAP running on the host system (Windows
>> 10).
> It would be good to report this to the WinPCAP people. I hate it when
> debug tools actually introduce bugs.
This will take me some time. I couldn't fairly blame WinPCAP. Firstly,
WinPCAP isn't really supported on Windows 10 (hence the fork,
Win10PCAP). Secondly, right now all that I know is that it's some
combination of WinPCAP and the virtual Intel 82540EM NIC. I'll have to
play around a bit; maybe try some of the other virtual NICs.
Then I think it would be good to have a more convenient test case than
"First, install netatalk 2.2.6." I'll see if I can make one.
Cheers,
Craig.
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Boris Brezillon @ 2018-08-21 5:44 UTC (permalink / raw)
To: Alban
Cc: Srinivas Kandagatla, Bartosz Golaszewski, Jonathan Corbet,
Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, Grygorii Strashko, David S . Miller, Naren,
Mauro Carvalho Chehab, Andrew Morton, Lukas Wunner, Dan Carpenter
In-Reply-To: <20180821005327.0d312a85@tock>
On Tue, 21 Aug 2018 00:53:27 +0200
Alban <albeu@free.fr> wrote:
> On Sun, 19 Aug 2018 18:46:09 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>
> > On Sun, 19 Aug 2018 13:31:06 +0200
> > Alban <albeu@free.fr> wrote:
> >
> > > On Fri, 17 Aug 2018 18:27:20 +0200
> > > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> > >
> > > > Hi Bartosz,
> > > >
> > > > On Fri, 10 Aug 2018 10:05:03 +0200
> > > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > > From: Alban Bedel <albeu@free.fr>
> > > > >
> > > > > Allow drivers that use the nvmem API to read data stored on MTD devices.
> > > > > For this the mtd devices are registered as read-only NVMEM providers.
> > > > >
> > > > > Signed-off-by: Alban Bedel <albeu@free.fr>
> > > > > [Bartosz:
> > > > > - use the managed variant of nvmem_register(),
> > > > > - set the nvmem name]
> > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > What happened to the 2 other patches of Alban's series? I'd really
> > > > like the DT case to be handled/agreed on in the same patchset, but
> > > > IIRC, Alban and Srinivas disagreed on how this should be represented.
> > > > I hope this time we'll come to an agreement, because the MTD <-> NVMEM
> > > > glue has been floating around for quite some time...
> > >
> > > These other patches were to fix what I consider a fundamental flaw in
> > > the generic NVMEM bindings, however we couldn't agree on this point.
> > > Bartosz later contacted me to take over this series and I suggested to
> > > just change the MTD NVMEM binding to use a compatible string on the
> > > NVMEM cells as an alternative solution to fix the clash with the old
> > > style MTD partition.
> > >
> > > However all this has no impact on the code needed to add NVMEM support
> > > to MTD, so the above patch didn't change at all.
> >
> > It does have an impact on the supported binding though.
> > nvmem->dev.of_node is automatically assigned to mtd->dev.of_node, which
> > means people will be able to define their NVMEM cells directly under
> > the MTD device and reference them from other nodes (even if it's not
> > documented), and as you said, it conflict with the old MTD partition
> > bindings. So we'd better agree on this binding before merging this
> > patch.
>
> Unless the nvmem cell node has a compatible string, then it won't be
> considered as a partition by the MTD code. That is were the clash is,
> both bindings allow free named child nodes without a compatible string.
Except the current nvmem cells parsing code does not enforce that, and
existing DTs rely on this behavior, so we're screwed. Or are you
suggesting to add a new "bool check_cells_compat;" field to
nvmem_config?
>
> > I see several options:
> >
> > 1/ provide a way to tell the NVMEM framework not to use parent->of_node
> > even if it's != NULL. This way we really don't support defining
> > NVMEM cells in the DT, and also don't support referencing the nvmem
> > device using a phandle.
>
> I really don't get what the point of this would be. Make the whole API
> useless?
No, just allow Bartosz to get his changes merged without waiting for you
and Srinivas to agree on how to handle the new binding. As I said
earlier, this mtd <-> nvmem stuff has been around for quite some time,
and instead of trying to find an approach that makes everyone happy, you
decided to let the patchset die.
>
> > 2/ define a new binding where all nvmem-cells are placed in an
> > "nvmem" subnode (just like we have this "partitions" subnode for
> > partitions), and then add a config->of_node field so that the
> > nvmem provider can explicitly specify the DT node representing the
> > nvmem device. We'll also need to set this field to ERR_PTR(-ENOENT)
> > in case this node does not exist so that the nvmem framework knows
> > that it should not assign nvmem->dev.of_node to parent->of_node
>
> This is not good. First the NVMEM device is only a virtual concept of
> the Linux kernel, it has no place in the DT.
nvmem-cells is a virtual concept too, still, you define them in the DT.
> Secondly the NVMEM
> provider (here the MTD device) then has to manually parse its DT node to
> find this subnode, pass it to the NVMEM framework to later again
> resolve it back to the MTD device.
We don't resolve it back to the MTD device, because the MTD device is
just the parent of the nvmem device.
> Not very complex but still a lot of
> useless code, just registering the MTD device is a lot simpler and much
> more inline with most other kernel API that register a "service"
> available from a device.
I'm not a big fan of this option either, but I thought I had to propose
it.
>
> > 3/ only declare partitions as nvmem providers. This would solve the
> > problem we have with partitions defined in the DT since
> > defining sub-partitions in the DT is not (yet?) supported and
> > partition nodes are supposed to be leaf nodes. Still, I'm not a big
> > fan of this solution because it will prevent us from supporting
> > sub-partitions if we ever want/need to.
>
> That sound like a poor workaround.
Yes, that's a workaround. And the reason I propose it, is, again,
because I don't want to block Bartosz.
> Remember that this problem could
> appear with any device that has a binding that use child nodes.
I'm talking about partitions, and you're talking about mtd devices.
Right now partitions don't have subnodes, and if we define that
partition subnodes should describe nvmem-cells, then it becomes part of
the official binding. So, no, the problem you mention does not (yet)
exist.
>
> > 4/ Add a ->of_xlate() hook that would be called if present by the
> > framework instead of using the default parsing we have right now.
>
> That is a bit cleaner, but I don't think it would be worse the
> complexity.
But it's way more flexible than putting everything in the nvmem
framework. BTW, did you notice that nvmem-cells parsing does not work
with flashes bigger than 4GB, because the framework assumes
#address-cells and #size-cells are always 1. That's probably something
we'll have to fix for the MTD case.
> Furthermore xlate functions are more about converting
> from hardware parameters to internal kernel representation than to hide
> extra DT parsing.
Hm, how is that different? ->of_xlate() is just a way for drivers to
have their own DT representation, which is exactly what we want here.
>
> > 5/ Tell the nvmem framework the name of the subnode containing nvmem
> > cell definitions (if NULL that means cells are directly defined
> > under the nvmem provider node). We would set it to "nvmem-cells" (or
> > whatever you like) for the MTD case.
>
> If so please match on compatible and not on the node name.
If you like.
>
> 6/ Extend the current NVMEM cell lookup to check if the parent node of
> the cell has a compatible string set to "nvmem-cells". If it doesn't it
> mean we have the current binding and this node is the NVMEM device. If
> it does the device node is just the next parent. This is trivial to
> implement (literally 2 lines of code) and cover all the cases currently
> known.
Except Srinivas was not happy with this solution, and this stalled the
discussion. I'm trying to find other options and you keep rejecting all
of them to come back to this one.
>
> 7/ Just add a compatible string to the nvmem cell. No code change is
> needed,
That's not true!!! What forces people to add this compatible in their
DT? Nothing. I'll tell you what will happen: people will start defining
their nvmem cells directly under the MTD node because that *works*, and
even if the binding is not documented and we consider it invalid, we'll
be stuck supporting it forever. As said above, the very reason for
option #1 to exist is to give you and Srinivas some more time to sort
this out, while unblocking Bartosz in the meantime.
> however as the nvmem cells have an address space (the offset in
> byte in the storage) it might still clash with another address space
> used by the main device biding (for example a number of child
> functions).
>
> > There are probably other options (some were proposed by Alban and
> > Srinivas already), but I'd like to get this sorted out before we merge
> > this patch.
> >
> > Alban, Srinivas, any opinion?
>
> My preference goes to 6/ as it is trivial to implement, solves all
> known shortcomings and is backward compatible with the current binding.
> All other solutions have limitations and/or require too complex
> implementations compared to what they try to solve.
So we're back to square 1, and you're again blocking everything because
you refuse to consider other options.
There's obviously nothing more I can do to help, and that's unfortunate
because other people are waiting for this feature.
Regards,
Boris
^ permalink raw reply
* Re: [PATCH] r8169: don't use MSI-X on RTL8106e
From: Marc Zyngier @ 2018-08-21 8:28 UTC (permalink / raw)
To: Bjorn Helgaas, Heiner Kallweit, jian-hong
Cc: David Miller, nic_swsd, netdev, linux-kernel, linux, linux-pci,
Thomas Gleixner, Christoph Hellwig
In-Reply-To: <20180820184438.GA154536@bhelgaas-glaptop.roam.corp.google.com>
On 20/08/18 19:44, Bjorn Helgaas wrote:
> [+cc Marc, Thomas, Christoph, linux-pci)
> (beginning of thread at [1])
>
> On Thu, Aug 16, 2018 at 09:50:48PM +0200, Heiner Kallweit wrote:
>> On 16.08.2018 21:39, David Miller wrote:
>>> From: Heiner Kallweit <hkallweit1@gmail.com>
>>> Date: Thu, 16 Aug 2018 21:37:31 +0200
>>>
>>>> On 16.08.2018 21:21, David Miller wrote:
>>>>> From: <jian-hong@endlessm.com>
>>>>> Date: Wed, 15 Aug 2018 14:21:10 +0800
>>>>>
>>>>>> Found the ethernet network on ASUS X441UAR doesn't come back on resume
>>>>>> from suspend when using MSI-X. The chip is RTL8106e - version 39.
>>>>>
>>>>> Heiner, please take a look at this.
>>>>>
>>>>> You recently disabled MSI-X on RTL8168g for similar reasons.
>>>>>
>>>>> Now that we've seen two chips like this, maybe there is some other
>>>>> problem afoot.
>>>>>
>>>> Thanks for the hint. I saw it already and just contacted Realtek
>>>> whether they are aware of any MSI-X issues with particular chip
>>>> versions. With the chip versions I have access to MSI-X works fine.
>>>>
>>>> There's also the theoretical option that the issues are caused by
>>>> broken BIOS's. But so far only chip versions have been reported
>>>> which are very similar, at least with regard to version number
>>>> (2x VER_40, 1x VER_39). So they may share some buggy component.
>>>>
>>>> Let's see whether Realtek can provide some hint.
>>>> If more chip versions are reported having problems with MSI-X,
>>>> then we could switch to a whitelist or disable MSI-X in general.
>>>
>>> It could be that we need to reprogram some register(s) on resume,
>>> which normally might not be needed, and that is what is causing the
>>> problem with some chips.
>>>
>> Indeed. That's what I'm checking with Realtek.
>> In the register list in the r8169 driver there's one entry which
>> seems to indicate that there are MSI-X specific settings.
>> However this register isn't used, and the r8168 vendor driver
>> uses only MSI. And there are no public datasheets.
>
> Do we have any information about these chip versions in other systems?
> Or other devices using MSI-X in the same ASUS system? It seems
> possible that there's some PCI core or suspend/resume issue with MSI-X
> and this patch just avoids it without fixing the root cause.
>
> It might be useful to have a kernel.org bugzilla with the complete
> dmesg, "sudo lspci -vv" output, and /proc/interrupts contents archived
> for future reference.
The one system I have with a Realtek chip seems happy enough with MSI-X,
but it never gets suspended. There is comment in the patch that I don't
quite get:
> It is the IRQ 127 - PCI-MSI used by enp2s0. However, lspci lists MSI is
> disabled and MSI-X is enabled which conflicts to the interrupt table.
What do you mean by "conflicts"? With what? Another question is whether
you've loaded any firmware (some versions of the Realtek HW seem to require
it).
For the posterity, some data from my own system, which I don't know if it
has any relevance to the problem at hand.
Thanks,
M.
[ 2.624963] r8169 0000:02:00.0 eth0: RTL8168g/8111g, 5a:fe:ad:ce:11:00, XID 4c000800, IRQ 26
[ 2.633398] r8169 0000:02:00.0 eth0: jumbo features [frames: 9200 bytes, tx checksumming: ko]
26: 50 997005 0 0 MSI 1048576 Edge enp2s0
02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)
Subsystem: Realtek Semiconductor Co., Ltd. RTL8111/8168 PCI Express Gigabit Ethernet controller
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 25
Region 0: I/O ports at 1000 [size=256]
Region 2: Memory at 100004000 (64-bit, prefetchable) [size=4K]
Region 4: Memory at 100000000 (64-bit, prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
Address: 0000000000000000 Data: 0000
Capabilities: [70] Express (v2) Endpoint, MSI 01
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- SlotPowerLimit 0.000W
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 4096 bytes
DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s unlimited, L1 <64us
ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via message/WAKE#
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
Compliance De-emphasis: -6dB
LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
Capabilities: [b0] MSI-X: Enable+ Count=4 Masked-
Vector table: BAR=4 offset=00000000
PBA: BAR=4 offset=00000800
Capabilities: [d0] Vital Product Data
pcilib: sysfs_read_vpd: read failed: Input/output error
Not readable
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr+ BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
Capabilities: [140 v1] Virtual Channel
Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
Arb: Fixed- WRR32- WRR64- WRR128-
Ctrl: ArbSelect=Fixed
Status: InProgress-
VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
Status: NegoPending- InProgress-
Capabilities: [160 v1] Device Serial Number 00-00-00-00-00-00-00-00
Capabilities: [170 v1] Latency Tolerance Reporting
Max snoop latency: 0ns
Max no snoop latency: 0ns
Kernel driver in use: r8169
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Charity Project
From: Wang Jianlin @ 2018-08-20 23:11 UTC (permalink / raw)
I intend to give you a portion of my wealth as a free-will financial
donation to you. Respond to partake.
Wang Jianlin
Dalian Wanda Group
^ permalink raw reply
* Re: [bpf-next RFC 0/3] Introduce eBPF flow dissector
From: David Miller @ 2018-08-21 2:24 UTC (permalink / raw)
To: alexei.starovoitov
Cc: peterpenkov96, netdev, ast, daniel, simon.horman, ppenkov,
willemb
In-Reply-To: <20180820205205.jj7e75pulwqkorpr@ast-mbp>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Mon, 20 Aug 2018 13:52:07 -0700
> I don't think copy-paste avoids the issue of uapi.
> Anything used by BPF program is uapi.
> The only exception is offsets of kernel internal structures
> passed into bpf_probe_read().
> So we have several options:
> 1. be honest and say 'struct flow_dissect_key*' is now uapi
> 2. wrap all of them into 'struct bpf_flow_dissect_key*' and do rewrites
> when/if 'struct flow_dissect_key*' changes
> 3. wait for BTF to solve it for tracing use case and for this one two.
...
> The idea is that kernel internal structs can be defined in bpf prog
> and since they will be described precisely in BTF that comes with the prog
> the kernel can validate that prog's BTF matches what kernel thinks it has.
> imo that's the most flexible, but BTF for all of vmlinux won't be ready
> tomorrow and looks like this patch set is ready to go, so I would go with 1 or 2.
I would definitely prefer #2 or #3.
I personally would like to see us avoid preventing interesting
optimizations of the flow key layout and/or accesses in the future.
^ permalink raw reply
* Re: unregister_netdevice: waiting for DEV to become free (2)
From: Cong Wang @ 2018-08-21 5:40 UTC (permalink / raw)
To: Julian Anastasov
Cc: syzbot+30209ea299c09d8785c9, ddstreet, Dmitry Vyukov, LKML,
Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <alpine.LFD.2.20.1808201527230.2758@ja.home.ssi.bg>
On Mon, Aug 20, 2018 at 6:00 AM Julian Anastasov <ja@ssi.bg> wrote:
>
>
> Hello,
>
> On Sun, 19 Aug 2018, syzbot wrote:
>
> > syzbot has found a reproducer for the following crash on:
> >
> > HEAD commit: d7857ae43dcc Add linux-next specific files for 20180817
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=13c72fce400000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=4b10cd1ea76bb092
> > dashboard link: https://syzkaller.appspot.com/bug?extid=30209ea299c09d8785c9
> > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15df679a400000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15242741400000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+30209ea299c09d8785c9@syzkaller.appspotmail.com
> >
> > IPVS: stopping master sync thread 4657 ...
> > IPVS: stopping master sync thread 4663 ...
> > IPVS: sync thread started: state = MASTER, mcast_ifn = syz_tun, syncid = 0, id
> > IPVS: = 0
> > IPVS: sync thread started: state = MASTER, mcast_ifn = syz_tun, syncid = 0, id
> > IPVS: = 0
> > IPVS: stopping master sync thread 4664 ...
> > unregister_netdevice: waiting for lo to become free. Usage count = 1
>
> Well, only IPVS and tun in the game? But IPVS does not
> take any dev references for sync threads. Can it be a problem
> in tun? For example, a side effects from dst_cache_reset?
> May be dst_release is called too late? Here is what should happen
> on unregistration:
There are multiple similar bugs grouped together under this, perhaps
they are different, perhaps they are a same bug, too early to say.
For the one I look into, dst_cache doesn't matter, because the xmit
path doesn't even use tunnel dst_cache at all, and it is ip6tnl0 FB
device, unlike this one which is tun device.
>
> - NETDEV_UNREGISTER event: rt_flush_dev changes dst->dev with lo
> but dst is not released
>
> - ndo_uninit/ip_tunnel_uninit: dst_cache_reset is called which
> does nothing!?! May be dst_release call is needed here.
I think this makes sense, at least prior to the general dst_cache
introduction, dst refcnt was released in ndo_uninit() too, so it
is reasonable to move the dst_cache_destroy() to ndo_uninit().
>
> - no more references are expected here ...
>
> - netdev_run_todo -> netdev_wait_allrefs: loop here due to refcnt!=0
>
> - dev->priv_destructor (ip_tunnel_dev_free) calls dst_cache_destroy
> where dst_release is used but it is not reached because we loop in
> netdev_wait_allrefs above
>
> - dst_cache_destroy: really call dst_release
>
> In fact, after calling rt_flush_dev and replacing the
> dst->dev we should reach dev->priv_destructor (ip_tunnel_dev_free)
> for tun device where dst_release for lo should be called. But may be
> something prevents it, exit batching?
I can't see anything in netnns exit batch is any special here.
For the one I look into, it seems some fib6_info is not released for
some reason. It seems to be the one created by addrconf_prefix_route(),
which is supposed to be released by fib6_clean_tree() I think, but it
never happens.
Thanks.
^ permalink raw reply
* Re: [PATCH] rhashtable: remove duplicated include from rhashtable.c
From: David Miller @ 2018-08-21 2:19 UTC (permalink / raw)
To: yuehaibing; +Cc: tgraf, herbert, netdev, kernel-janitors
In-Reply-To: <1534815716-102071-1-git-send-email-yuehaibing@huawei.com>
From: Yue Haibing <yuehaibing@huawei.com>
Date: Tue, 21 Aug 2018 01:41:56 +0000
> Remove duplicated include.
>
> Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
Applied, thank you.
^ permalink raw reply
* Re: KASAN: use-after-free Read in tipc_group_fill_sock_diag
From: Cong Wang @ 2018-08-21 5:29 UTC (permalink / raw)
To: syzbot+b9c8f3ab2994b7cd1625
Cc: David Miller, Jon Maloy, LKML, Linux Kernel Network Developers,
syzkaller-bugs, tipc-discussion, Ying Xue
In-Reply-To: <00000000000038cc590573be9db2@google.com>
On Sat, Aug 18, 2018 at 5:10 PM syzbot
<syzbot+b9c8f3ab2994b7cd1625@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: a18d783fedfe Merge tag 'driver-core-4.19-rc1' of git://git..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=14bcc936400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=7eeabb17332898c0
> dashboard link: https://syzkaller.appspot.com/bug?extid=b9c8f3ab2994b7cd1625
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=1374ec4a400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+b9c8f3ab2994b7cd1625@syzkaller.appspotmail.com
>
> IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> 8021q: adding VLAN 0 to HW filter on device team0
> ==================================================================
> BUG: KASAN: use-after-free in tipc_group_fill_sock_diag+0x7b9/0x84b
> net/tipc/group.c:921
> Read of size 4 at addr ffff8801ceb5565c by task syz-executor0/4838
>
> CPU: 1 PID: 4838 Comm: syz-executor0 Not tainted 4.18.0+ #196
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> print_address_description+0x6c/0x20b mm/kasan/report.c:256
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
> __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
> tipc_group_fill_sock_diag+0x7b9/0x84b net/tipc/group.c:921
> tipc_sk_fill_sock_diag+0x9f8/0xdb0 net/tipc/socket.c:3322
> __tipc_add_sock_diag+0x22f/0x360 net/tipc/diag.c:62
> tipc_nl_sk_walk+0x68d/0xd30 net/tipc/socket.c:3249
> tipc_diag_dump+0x24/0x30 net/tipc/diag.c:73
Seems we need just unlink the tipc sock before deleting group,
otherwise probably have to take a refcnt of group.
For example, moving tipc_sk_leave() down after tipc_sk_remove()?
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -571,10 +571,10 @@ static int tipc_release(struct socket *sock)
__tipc_shutdown(sock, TIPC_ERR_NO_PORT);
sk->sk_shutdown = SHUTDOWN_MASK;
- tipc_sk_leave(tsk);
tipc_sk_withdraw(tsk, 0, NULL);
sk_stop_timer(sk, &sk->sk_timer);
tipc_sk_remove(tsk);
+ tipc_sk_leave(tsk);
/* Reject any messages that accumulated in backlog queue */
release_sock(sk);
> netlink_dump+0x519/0xd50 net/netlink/af_netlink.c:2233
> __netlink_dump_start+0x4f1/0x6f0 net/netlink/af_netlink.c:2329
> netlink_dump_start include/linux/netlink.h:213 [inline]
> tipc_sock_diag_handler_dump+0x234/0x340 net/tipc/diag.c:89
> __sock_diag_cmd net/core/sock_diag.c:232 [inline]
> sock_diag_rcv_msg+0x31d/0x410 net/core/sock_diag.c:263
> netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2454
> sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:274
> netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
> netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1343
> netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1908
> sock_sendmsg_nosec net/socket.c:621 [inline]
> sock_sendmsg+0xd5/0x120 net/socket.c:631
> ___sys_sendmsg+0x7fd/0x930 net/socket.c:2114
> __sys_sendmsg+0x11d/0x290 net/socket.c:2152
> __do_sys_sendmsg net/socket.c:2161 [inline]
> __se_sys_sendmsg net/socket.c:2159 [inline]
> __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x457089
> Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f3a5d506c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> RAX: ffffffffffffffda RBX: 00007f3a5d5076d4 RCX: 0000000000457089
> RDX: 0000000000000000 RSI: 0000000020000040 RDI: 0000000000000006
> RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 00000000004d4088 R14: 00000000004c8ab0 R15: 0000000000000000
>
> Allocated by task 4838:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
> kmem_cache_alloc_trace+0x152/0x780 mm/slab.c:3620
> kmalloc include/linux/slab.h:513 [inline]
> kzalloc include/linux/slab.h:707 [inline]
> tipc_group_create+0x155/0xa70 net/tipc/group.c:171
> tipc_sk_join net/tipc/socket.c:2766 [inline]
> tipc_setsockopt+0x2d1/0xd70 net/tipc/socket.c:2881
> __sys_setsockopt+0x1c5/0x3b0 net/socket.c:1900
> __do_sys_setsockopt net/socket.c:1911 [inline]
> __se_sys_setsockopt net/socket.c:1908 [inline]
> __x64_sys_setsockopt+0xbe/0x150 net/socket.c:1908
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 4837:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
> kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
> __cache_free mm/slab.c:3498 [inline]
> kfree+0xd9/0x260 mm/slab.c:3813
> tipc_group_delete+0x2e5/0x3f0 net/tipc/group.c:227
> tipc_sk_leave+0x113/0x220 net/tipc/socket.c:2800
> tipc_release+0x14e/0x12b0 net/tipc/socket.c:574
> __sock_release+0xd7/0x250 net/socket.c:579
> sock_close+0x19/0x20 net/socket.c:1139
> __fput+0x39b/0x860 fs/file_table.c:251
> ____fput+0x15/0x20 fs/file_table.c:282
> task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
> tracehook_notify_resume include/linux/tracehook.h:193 [inline]
> exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166
> prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
> syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
> do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8801ceb55600
> which belongs to the cache kmalloc-192 of size 192
> The buggy address is located 92 bytes inside of
> 192-byte region [ffff8801ceb55600, ffff8801ceb556c0)
> The buggy address belongs to the page:
> page:ffffea00073ad540 count:1 mapcount:0 mapping:ffff8801dac00040 index:0x0
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffffea00073a5088 ffffea00073ae5c8 ffff8801dac00040
> raw: 0000000000000000 ffff8801ceb55000 0000000100000010 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8801ceb55500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8801ceb55580: 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8801ceb55600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ^
> ffff8801ceb55680: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> ffff8801ceb55700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Boris Brezillon @ 2018-08-21 5:07 UTC (permalink / raw)
To: Alban, Srinivas Kandagatla, Rob Herring
Cc: Bartosz Golaszewski, Jonathan Corbet, Sekhar Nori, Kevin Hilman,
Russell King, Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse,
Brian Norris, Marek Vasut, Richard Weinberger, Grygorii Strashko,
David S . Miller, Naren, Mauro Carvalho Chehab, Andrew Morton,
Lukas Wunner, Dan Carpenter, Florian Fainelli, Ivan
In-Reply-To: <20180820232748.608c3667@tock>
On Mon, 20 Aug 2018 23:27:48 +0200
Alban <albeu@free.fr> wrote:
> On Mon, 20 Aug 2018 20:20:38 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>
> > On Mon, 20 Aug 2018 11:43:34 +0100
> > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> >
> > >
> > > Overall am still not able to clear visualize on how MTD bindings with
> > > nvmem cells would look in both partition and un-partition usecases?
> > > An example DT would be nice here!!
> >
> > Something along those lines:
>
> We must also have a compatible string on the nvmem-cells node to make
> sure we don't clash with the old style MTD partitions,
That's not possible, because we don't have a reg prop in the
nvmem-cells node.
> or some other
> device specific binding.
This one might happen.
Was Rob okay with this compatible? If he was, I guess we can go for
this binding. Srinivas, any objection?
>
> >
> > mtdnode {
> > nvmem-cells {
> compatible = "nvmem-cells";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x14>;
> > };
> > };
> >
> > partitions {
> > compatible = "fixed-partitions";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > partition@0 {
> > reg = <0x0 0x20000>;
> >
> > nvmem-cells {
> compatible = "nvmem-cells";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x10>;
> > };
> > };
> > };
> > };
> > };
>
>
> Alban
^ permalink raw reply
* [PATCH] rhashtable: remove duplicated include from rhashtable.c
From: Yue Haibing @ 2018-08-21 1:41 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: Yue Haibing, netdev, kernel-janitors
Remove duplicated include.
Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
---
lib/rhashtable.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index ae4223e..672eecd 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -28,7 +28,6 @@
#include <linux/rhashtable.h>
#include <linux/err.h>
#include <linux/export.h>
-#include <linux/rhashtable.h>
#define HASH_DEFAULT_SIZE 64UL
#define HASH_MIN_SIZE 4U
^ permalink raw reply related
* Re: [PATCH net] net/ipv6: Put lwtstate when destroying fib6_info
From: David Miller @ 2018-08-21 1:19 UTC (permalink / raw)
To: dsahern; +Cc: netdev, dsahern
In-Reply-To: <20180820200241.19876-1-dsahern@kernel.org>
From: dsahern@kernel.org
Date: Mon, 20 Aug 2018 13:02:41 -0700
> From: David Ahern <dsahern@gmail.com>
>
> Prior to the introduction of fib6_info lwtstate was managed by the dst
> code. With fib6_info releasing lwtstate needs to be done when the struct
> is freed.
>
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied and queued up for -stable, thanks David.
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Stephen Hemminger @ 2018-08-21 0:52 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: Mahesh Bandewar, netdev
In-Reply-To: <CAF2d9jh3xq_F799Dn0=R0XK7NAkrQxmREg++sT4e3MpVLsnPpw@mail.gmail.com>
On Mon, 20 Aug 2018 16:44:28 -0700
Mahesh Bandewar (महेश बंडेवार) <maheshb@google.com> wrote:
> On Mon, Aug 20, 2018 at 4:38 PM, Mahesh Bandewar (महेश बंडेवार)
> <maheshb@google.com> wrote:
> > On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> >> On Mon, 20 Aug 2018 14:42:15 -0700
> >> Mahesh Bandewar <mahesh@bandewar.net> wrote:
> >>
> >>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
> >>> index ace4b3dd738b..a524b520b276 100644
> >>> --- a/tc/m_ematch.c
> >>> +++ b/tc/m_ematch.c
> >>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
> >>> return count;
> >>> }
> >>>
> >>> +__attribute__((format(printf, 5, 6)))
> >>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
> >>> struct ematch_util *e, char *fmt, ...)
> >>
> >> I think the printf attribute needs to go on the function prototype
> >> here:
> >> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
> >>
> > The attributes are attached to the definitions only and not prototype
> > declarations. Please see the definition/declaration for jsonw_printf()
> > in the same patch.
> I take that back. Seems like it's fine either way.
The reason to put the attributes in the .h file is that then the compiler
can test for misuse in other files. For example if em_parse_error had
a bad format in em_u32.c, then the warning would not happen unless
the attribute was on the function prototype.
^ permalink raw reply
* Re: [PATCH net-next v8 7/7] net: vhost: make busyloop_intr more accurate
From: Jason Wang @ 2018-08-21 0:47 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: netdev, virtualization
In-Reply-To: <f85bfa97-ab9c-2d51-2053-1fe6bb3d45bc@redhat.com>
On 2018年08月21日 08:33, Jason Wang wrote:
>
>
> On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> The patch uses vhost_has_work_pending() to check if
>> the specified handler is scheduled, because in the most case,
>> vhost_has_work() return true when other side handler is added
>> to worker list. Use the vhost_has_work_pending() insead of
>> vhost_has_work().
>>
>> Topology:
>> [Host] ->linux bridge -> tap vhost-net ->[Guest]
>>
>> TCP_STREAM (netperf):
>> * Without the patch: 38035.39 Mbps, 3.37 us mean latency
>> * With the patch: 38409.44 Mbps, 3.34 us mean latency
>
> The improvement is not obvious as last version. Do you imply there's
> some recent changes of vhost that make it faster?
>
I misunderstood the numbers, please ignore this.
It shows less than 1% improvement. I'm not sure it's worth to do so. Can
you try bi-directional pktgen to see if it has more obvious effect?
Thanks
> Thanks
>
>>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> ---
>> drivers/vhost/net.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index db63ae2..b6939ef 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -487,10 +487,8 @@ static void vhost_net_busy_poll(struct vhost_net
>> *net,
>> endtime = busy_clock() + busyloop_timeout;
>> while (vhost_can_busy_poll(endtime)) {
>> - if (vhost_has_work(&net->dev)) {
>> - *busyloop_intr = true;
>> + if (vhost_has_work(&net->dev))
>> break;
>> - }
>> if ((sock_has_rx_data(sock) &&
>> !vhost_vq_avail_empty(&net->dev, rvq)) ||
>> @@ -513,6 +511,11 @@ static void vhost_net_busy_poll(struct vhost_net
>> *net,
>> !vhost_has_work_pending(&net->dev, VHOST_NET_VQ_RX))
>> vhost_net_enable_vq(net, rvq);
>> + if (vhost_has_work_pending(&net->dev,
>> + poll_rx ?
>> + VHOST_NET_VQ_RX: VHOST_NET_VQ_TX))
>> + *busyloop_intr = true;
>> +
>> mutex_unlock(&vq->mutex);
>> }
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v8 5/7] net: vhost: introduce bitmap for vhost_poll
From: Jason Wang @ 2018-08-21 0:45 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: virtualization, netdev
In-Reply-To: <1534680686-3108-6-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The bitmap of vhost_dev can help us to check if the
> specified poll is scheduled. This patch will be used
> for next two patches.
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/vhost/net.c | 11 +++++++++--
> drivers/vhost/vhost.c | 17 +++++++++++++++--
> drivers/vhost/vhost.h | 7 ++++++-
> 3 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 1eff72d..23d7ffc 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1135,8 +1135,15 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> }
> vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX);
>
> - vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev);
> - vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev);
> + vhost_poll_init(n->poll + VHOST_NET_VQ_TX,
> + handle_tx_net,
> + VHOST_NET_VQ_TX,
> + EPOLLOUT, dev);
> +
> + vhost_poll_init(n->poll + VHOST_NET_VQ_RX,
> + handle_rx_net,
> + VHOST_NET_VQ_RX,
> + EPOLLIN, dev);
>
> f->private_data = n;
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a1c06e7..dc88a60 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -186,7 +186,7 @@ void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
>
> /* Init poll structure */
> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> - __poll_t mask, struct vhost_dev *dev)
> + __u8 poll_id, __poll_t mask, struct vhost_dev *dev)
> {
> init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
> init_poll_funcptr(&poll->table, vhost_poll_func);
> @@ -194,6 +194,7 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> poll->dev = dev;
> poll->wqh = NULL;
>
> + poll->poll_id = poll_id;
> vhost_work_init(&poll->work, fn);
> }
> EXPORT_SYMBOL_GPL(vhost_poll_init);
> @@ -276,8 +277,16 @@ bool vhost_has_work(struct vhost_dev *dev)
> }
> EXPORT_SYMBOL_GPL(vhost_has_work);
>
> +bool vhost_has_work_pending(struct vhost_dev *dev, int poll_id)
> +{
> + return !llist_empty(&dev->work_list) &&
> + test_bit(poll_id, dev->work_pending);
I think we've already had something similar. E.g can we test
VHOST_WORK_QUEUED instead?
Thanks
> +}
> +EXPORT_SYMBOL_GPL(vhost_has_work_pending);
> +
> void vhost_poll_queue(struct vhost_poll *poll)
> {
> + set_bit(poll->poll_id, poll->dev->work_pending);
> vhost_work_queue(poll->dev, &poll->work);
> }
> EXPORT_SYMBOL_GPL(vhost_poll_queue);
> @@ -354,6 +363,7 @@ static int vhost_worker(void *data)
> if (!node)
> schedule();
>
> + bitmap_zero(dev->work_pending, VHOST_DEV_MAX_VQ);
> node = llist_reverse_order(node);
> /* make sure flag is seen after deletion */
> smp_wmb();
> @@ -420,6 +430,8 @@ void vhost_dev_init(struct vhost_dev *dev,
> struct vhost_virtqueue *vq;
> int i;
>
> + BUG_ON(nvqs > VHOST_DEV_MAX_VQ);
> +
> dev->vqs = vqs;
> dev->nvqs = nvqs;
> mutex_init(&dev->mutex);
> @@ -428,6 +440,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> dev->iotlb = NULL;
> dev->mm = NULL;
> dev->worker = NULL;
> + bitmap_zero(dev->work_pending, VHOST_DEV_MAX_VQ);
> init_llist_head(&dev->work_list);
> init_waitqueue_head(&dev->wait);
> INIT_LIST_HEAD(&dev->read_list);
> @@ -445,7 +458,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> vhost_vq_reset(dev, vq);
> if (vq->handle_kick)
> vhost_poll_init(&vq->poll, vq->handle_kick,
> - EPOLLIN, dev);
> + i, EPOLLIN, dev);
> }
> }
> EXPORT_SYMBOL_GPL(vhost_dev_init);
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 6c844b9..60b6f6d 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -30,6 +30,7 @@ struct vhost_poll {
> wait_queue_head_t *wqh;
> wait_queue_entry_t wait;
> struct vhost_work work;
> + __u8 poll_id;
> __poll_t mask;
> struct vhost_dev *dev;
> };
> @@ -37,9 +38,10 @@ struct vhost_poll {
> void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
> void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> bool vhost_has_work(struct vhost_dev *dev);
> +bool vhost_has_work_pending(struct vhost_dev *dev, int poll_id);
>
> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> - __poll_t mask, struct vhost_dev *dev);
> + __u8 id, __poll_t mask, struct vhost_dev *dev);
> int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> void vhost_poll_stop(struct vhost_poll *poll);
> void vhost_poll_flush(struct vhost_poll *poll);
> @@ -152,6 +154,8 @@ struct vhost_msg_node {
> struct list_head node;
> };
>
> +#define VHOST_DEV_MAX_VQ 128
> +
> struct vhost_dev {
> struct mm_struct *mm;
> struct mutex mutex;
> @@ -159,6 +163,7 @@ struct vhost_dev {
> int nvqs;
> struct eventfd_ctx *log_ctx;
> struct llist_head work_list;
> + DECLARE_BITMAP(work_pending, VHOST_DEV_MAX_VQ);
> struct task_struct *worker;
> struct vhost_umem *umem;
> struct vhost_umem *iotlb;
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-21 0:33 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <CAF2d9jh3xq_F799Dn0=R0XK7NAkrQxmREg++sT4e3MpVLsnPpw@mail.gmail.com>
On Mon, Aug 20, 2018 at 4:44 PM, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> On Mon, Aug 20, 2018 at 4:38 PM, Mahesh Bandewar (महेश बंडेवार)
> <maheshb@google.com> wrote:
>> On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
>> <stephen@networkplumber.org> wrote:
>>> On Mon, 20 Aug 2018 14:42:15 -0700
>>> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>>>
>>>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
>>>> index ace4b3dd738b..a524b520b276 100644
>>>> --- a/tc/m_ematch.c
>>>> +++ b/tc/m_ematch.c
>>>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
>>>> return count;
>>>> }
>>>>
>>>> +__attribute__((format(printf, 5, 6)))
>>>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>>> struct ematch_util *e, char *fmt, ...)
>>>
>>> I think the printf attribute needs to go on the function prototype
>>> here:
>>> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>>
>> The attributes are attached to the definitions only and not prototype
>> declarations. Please see the definition/declaration for jsonw_printf()
>> in the same patch.
> I take that back. Seems like it's fine either way.
>>>
>>> PS: I need to take the extern of those function prototypes.
I can take care of this in v2
^ permalink raw reply
* Re: [PATCH net-next v8 7/7] net: vhost: make busyloop_intr more accurate
From: Jason Wang @ 2018-08-21 0:33 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: virtualization, netdev
In-Reply-To: <1534680686-3108-8-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The patch uses vhost_has_work_pending() to check if
> the specified handler is scheduled, because in the most case,
> vhost_has_work() return true when other side handler is added
> to worker list. Use the vhost_has_work_pending() insead of
> vhost_has_work().
>
> Topology:
> [Host] ->linux bridge -> tap vhost-net ->[Guest]
>
> TCP_STREAM (netperf):
> * Without the patch: 38035.39 Mbps, 3.37 us mean latency
> * With the patch: 38409.44 Mbps, 3.34 us mean latency
The improvement is not obvious as last version. Do you imply there's
some recent changes of vhost that make it faster?
Thanks
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/vhost/net.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index db63ae2..b6939ef 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -487,10 +487,8 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> endtime = busy_clock() + busyloop_timeout;
>
> while (vhost_can_busy_poll(endtime)) {
> - if (vhost_has_work(&net->dev)) {
> - *busyloop_intr = true;
> + if (vhost_has_work(&net->dev))
> break;
> - }
>
> if ((sock_has_rx_data(sock) &&
> !vhost_vq_avail_empty(&net->dev, rvq)) ||
> @@ -513,6 +511,11 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> !vhost_has_work_pending(&net->dev, VHOST_NET_VQ_RX))
> vhost_net_enable_vq(net, rvq);
>
> + if (vhost_has_work_pending(&net->dev,
> + poll_rx ?
> + VHOST_NET_VQ_RX: VHOST_NET_VQ_TX))
> + *busyloop_intr = true;
> +
> mutex_unlock(&vq->mutex);
> }
>
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-21 0:32 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <20180820155038.30d0f08c@xeon-e3>
On Mon, Aug 20, 2018 at 3:50 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Mon, 20 Aug 2018 14:42:15 -0700
> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>
>>
>> if (is_json_context()) {
>> + json_writer_t *jw;
>> +
>> open_json_object("bittiming");
>> print_int(PRINT_ANY, "bitrate", NULL, bt->bitrate);
>> - jsonw_float_field_fmt(get_json_writer(),
>> - "sample_point", "%.3f",
>> - (float) bt->sample_point / 1000.);
>> + jw = get_json_writer();
>> + jsonw_name(jw, "sample_point");
>> + jsonw_printf(jw, "%.3f",
>> + (float) bt->sample_point / 1000);
>
> I think it would be better to get rid of the is_json_context() here in the CAN code
> and just use the print_json functions completely. Most of the other code is able to
> do that already.
Seems like this is not the only location and need to be taken care
every where in the CAN code. I'll leave it to some JSON / print expert
^ permalink raw reply
* Re: [PATCH net-next,v4] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: David Miller @ 2018-08-21 0:30 UTC (permalink / raw)
To: doronrk; +Cc: davejwatson, vakul.garg, borisp, aviadye, netdev
In-Reply-To: <20180821002723.GA79644@doronrk-mbp>
From: Doron Roberts-Kedes <doronrk@fb.com>
Date: Mon, 20 Aug 2018 17:27:23 -0700
> Given that frag_lists are not unlikely in this case, I believe the only
> remaining feedback on the original patch was the recursive
> implementation. If you'd like, I can re-submit with an iterative
> implementation, but I noticed that goes against the existing recursive
> pattern in functions like skb_release_data -> kfree_skb_list -> kfree_skb
> -> __kfree_skb -> skb_release_all -> skb_release_data, as well as
> skb_to_sgvec. Let me know whether an iterative implementation is
> preferred here, or whether I can simply rebase and resubmit a patch
> similar to the original (modulo some variable renaming improvements).
Ok, I guess staying with the recursive implementation is fine.
It's a real shame that frag lists are so common in this code path,
especially nested ones :-/
In the long term, perhaps we can do something about that.
In the short term, I guess this means your original change is OK.
Please resubmit when the net-next tree opens back up, thanks.
^ permalink raw reply
* Re: [PATCH net-next,v4] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: Doron Roberts-Kedes @ 2018-08-21 0:27 UTC (permalink / raw)
To: David Miller; +Cc: davejwatson, vakul.garg, borisp, aviadye, netdev
In-Reply-To: <20180811.115453.2016294695634012705.davem@davemloft.net>
On Sat, Aug 11, 2018 at 11:54:53AM -0700, David Miller wrote:
> From: Doron Roberts-Kedes <doronrk@fb.com>
> Date: Thu, 9 Aug 2018 15:43:44 -0700
>
> The reason is that we usually never need to "map" an SKB on receive,
> and on transmit the SKB geometry is normalized by the destination
> device features which means no frag_list.
>
> And the other existing receive path doing SW crypto, IPSEC, always
> COWs the data so get the number of SG array entries needed from
> skb_cow_data().
Makes sense, thanks!
> Frag lists on RX should be pretty rare. They occur when GRO
> segmentation encouters poorly arranged incoming SKBs. For example
> this happens if the incoming frames use lots of tiny SKB page frags,
> and therefore prevent accumulation into the page frag array of the
> head GRO skb.
>
> So yes it can happen, and we have to account for it.
>
> So I guess you really do need to accomodate this situation. Why
> don't you try to rearrange your new function with some likely()
> and unlikely() tests so that the straight code path optimizes for
> the non-frag-list case?
So I did some poking around and found that basically 100% of skb's
recieved by ktls have a frag_list because of how strparser is
implemented. skb's from TCP that do not a have a frag_list are
accumulated by strparser using frag_list of a new skb. skb's from TCP
that do have a frag_list can become part of skb's with nested frag_lists
(skb's with non-NULL frag_list that are themselves part of a frag_list).
Unfortunatley, frag_list seems to be the common case, so is probably not a
good candidate for an unlikely() test.
Regarding nested frag_list's more generally, is a good rule of thumb
that multiple layers of frag_list will not occur except for
post-processing such as in strparser? When should skb-handling code be
prepared for nested frag_lists?
Given that frag_lists are not unlikely in this case, I believe the only
remaining feedback on the original patch was the recursive
implementation. If you'd like, I can re-submit with an iterative
implementation, but I noticed that goes against the existing recursive
pattern in functions like skb_release_data -> kfree_skb_list -> kfree_skb
-> __kfree_skb -> skb_release_all -> skb_release_data, as well as
skb_to_sgvec. Let me know whether an iterative implementation is
preferred here, or whether I can simply rebase and resubmit a patch
similar to the original (modulo some variable renaming improvements).
^ permalink raw reply
* Re: [PATCH net-next v8 3/7] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Jason Wang @ 2018-08-21 3:15 UTC (permalink / raw)
To: xiangxia.m.yue, mst, makita.toshiaki; +Cc: netdev, virtualization
In-Reply-To: <1534680686-3108-4-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Factor out generic busy polling logic and will be
> used for in tx path in the next patch. And with the patch,
> qemu can set differently the busyloop_timeout for rx queue.
>
> To avoid duplicate codes, introduce the helper functions:
> * sock_has_rx_data(changed from sk_has_rx_data)
> * vhost_net_busy_poll_try_queue
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/vhost/net.c | 111 +++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 71 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 32c1b52..453c061 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -440,6 +440,75 @@ static void vhost_net_signal_used(struct vhost_net_virtqueue *nvq)
> nvq->done_idx = 0;
> }
>
> +static int sock_has_rx_data(struct socket *sock)
> +{
> + if (unlikely(!sock))
> + return 0;
> +
> + if (sock->ops->peek_len)
> + return sock->ops->peek_len(sock);
> +
> + return skb_queue_empty(&sock->sk->sk_receive_queue);
> +}
> +
> +static void vhost_net_busy_poll_try_queue(struct vhost_net *net,
> + struct vhost_virtqueue *vq)
> +{
> + if (!vhost_vq_avail_empty(&net->dev, vq)) {
> + vhost_poll_queue(&vq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> + vhost_disable_notify(&net->dev, vq);
> + vhost_poll_queue(&vq->poll);
> + }
> +}
> +
> +static void vhost_net_busy_poll(struct vhost_net *net,
> + struct vhost_virtqueue *rvq,
> + struct vhost_virtqueue *tvq,
> + bool *busyloop_intr,
> + bool poll_rx)
> +{
> + unsigned long busyloop_timeout;
> + unsigned long endtime;
> + struct socket *sock;
> + struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
> +
> + mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
> + vhost_disable_notify(&net->dev, vq);
> + sock = rvq->private_data;
> +
> + busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
> + tvq->busyloop_timeout;
> +
> + preempt_disable();
> + endtime = busy_clock() + busyloop_timeout;
> +
> + while (vhost_can_busy_poll(endtime)) {
> + if (vhost_has_work(&net->dev)) {
> + *busyloop_intr = true;
> + break;
> + }
> +
> + if ((sock_has_rx_data(sock) &&
> + !vhost_vq_avail_empty(&net->dev, rvq)) ||
> + !vhost_vq_avail_empty(&net->dev, tvq))
> + break;
> +
> + cpu_relax();
> + }
> +
> + preempt_enable();
> +
> + if (poll_rx)
> + vhost_net_busy_poll_try_queue(net, tvq);
> + else if (sock_has_rx_data(sock))
> + vhost_net_busy_poll_try_queue(net, rvq);
This could be simplified like:
if (poll_rx || sock_has_rx_data(sock))
vhost_net_busy_poll_try_queue(net, vq);
Thanks
> + else /* On tx here, sock has no rx data. */
> + vhost_enable_notify(&net->dev, rvq);
> +
> + mutex_unlock(&vq->mutex);
> +}
> +
> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> struct vhost_net_virtqueue *nvq,
> unsigned int *out_num, unsigned int *in_num,
> @@ -753,16 +822,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
> return len;
> }
>
> -static int sk_has_rx_data(struct sock *sk)
> -{
> - struct socket *sock = sk->sk_socket;
> -
> - if (sock->ops->peek_len)
> - return sock->ops->peek_len(sock);
> -
> - return skb_queue_empty(&sk->sk_receive_queue);
> -}
> -
> static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> bool *busyloop_intr)
> {
> @@ -770,41 +829,13 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
> struct vhost_virtqueue *rvq = &rnvq->vq;
> struct vhost_virtqueue *tvq = &tnvq->vq;
> - unsigned long uninitialized_var(endtime);
> int len = peek_head_len(rnvq, sk);
>
> - if (!len && tvq->busyloop_timeout) {
> + if (!len && rvq->busyloop_timeout) {
> /* Flush batched heads first */
> vhost_net_signal_used(rnvq);
> /* Both tx vq and rx socket were polled here */
> - mutex_lock_nested(&tvq->mutex, VHOST_NET_VQ_TX);
> - vhost_disable_notify(&net->dev, tvq);
> -
> - preempt_disable();
> - endtime = busy_clock() + tvq->busyloop_timeout;
> -
> - while (vhost_can_busy_poll(endtime)) {
> - if (vhost_has_work(&net->dev)) {
> - *busyloop_intr = true;
> - break;
> - }
> - if ((sk_has_rx_data(sk) &&
> - !vhost_vq_avail_empty(&net->dev, rvq)) ||
> - !vhost_vq_avail_empty(&net->dev, tvq))
> - break;
> - cpu_relax();
> - }
> -
> - preempt_enable();
> -
> - if (!vhost_vq_avail_empty(&net->dev, tvq)) {
> - vhost_poll_queue(&tvq->poll);
> - } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
> - vhost_disable_notify(&net->dev, tvq);
> - vhost_poll_queue(&tvq->poll);
> - }
> -
> - mutex_unlock(&tvq->mutex);
> + vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, true);
>
> len = peek_head_len(rnvq, sk);
> }
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Patch net 8/9] act_ife: move tcfa_lock down to where necessary
From: Cong Wang @ 2018-08-20 23:57 UTC (permalink / raw)
To: David Miller
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Vlad Buslov
In-Reply-To: <20180820.112952.2052634913677104782.davem@davemloft.net>
On Mon, Aug 20, 2018 at 11:29 AM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Sun, 19 Aug 2018 12:22:12 -0700
>
> > The only time we need to take tcfa_lock is when adding
> > a new metainfo to an existing ife->metalist. We don't need
> > to take tcfa_lock so early and so broadly in tcf_ife_init().
> >
> > This means we can always take ife_mod_lock first, avoid the
> > reverse locking ordering warning as reported by Vlad.
> >
> > Reported-by: Vlad Buslov <vladbu@mellanox.com>
> > Tested-by: Vlad Buslov <vladbu@mellanox.com>
> > Cc: Vlad Buslov <vladbu@mellanox.com>
> > Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> After this change we no longer call populate_metalist() in an atomic
> context via tcf_ife_init(), and populate_metalist passes 'exists'
> down to add_metainfo() as an 'atomic' indicator. It doesn't have this
> meaning if you aren't holding the tcfa_lock in the callers with BH
> disabled.
Passing 'exists' as 'atomic' is prior to my change. With my change,
they are separated as two parameters:
static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval,
- int len, bool atomic)
+ int len, bool atomic, bool exists)
Or I misunderstand your point here?
>
> Therefore, add_metainfo()'s 'atomic' indication is inaccurate in this
> call chain and will use GFP_ATOMIC unnecessarily.
>
> Probably the thing to just is just pass 'false' down to add_metainfo()
> in populate_metalist().
This is exactly what this patch does?
static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb,
bool exists, bool rtnl_held)
{
@@ -436,12 +431,11 @@ static int populate_metalist(struct tcf_ife_info
*ife, struct nlattr **tb,
...
- rc = add_metainfo(ife, i, val, len, exists);
+ rc = add_metainfo(ife, i, val, len, false, exists);
^ permalink raw reply
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-20 23:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <CAF2d9jj5EpLjGgpe94vYj9jt-MJHyZEcNaLOUTtYtibgEBpzww@mail.gmail.com>
On Mon, Aug 20, 2018 at 4:38 PM, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>> On Mon, 20 Aug 2018 14:42:15 -0700
>> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>>
>>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
>>> index ace4b3dd738b..a524b520b276 100644
>>> --- a/tc/m_ematch.c
>>> +++ b/tc/m_ematch.c
>>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
>>> return count;
>>> }
>>>
>>> +__attribute__((format(printf, 5, 6)))
>>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>> struct ematch_util *e, char *fmt, ...)
>>
>> I think the printf attribute needs to go on the function prototype
>> here:
>> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>>
> The attributes are attached to the definitions only and not prototype
> declarations. Please see the definition/declaration for jsonw_printf()
> in the same patch.
I take that back. Seems like it's fine either way.
>>
>> PS: I need to take the extern of those function prototypes.
^ permalink raw reply
* [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-08-20 23:43 UTC (permalink / raw)
To: Linus Walleij
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
David S. Miller, Dominik Brodowski, Kishon Vijay Abraham I,
Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
Jiri Slaby, linux-gpio, linux-doc, linux-i2c
In-Reply-To: <20180820234341.5271-1-jmkrzyszt@gmail.com>
Most users of get/set array functions iterate consecutive bits of data,
usually a single integer, while or processing array of results obtained
from or building an array of values to be passed to those functions.
Save time wasted on those iterations by changing the functions' API to
accept bitmaps.
All current users are updated as well.
More benefits from the change are expected as soon as planned support
for accepting/passing those bitmaps directly from/to respective GPIO
chip callbacks if applicable is implemented.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Documentation/driver-api/gpio/consumer.rst | 22 ++++----
drivers/auxdisplay/hd44780.c | 52 +++++++++--------
drivers/bus/ts-nbus.c | 19 ++-----
drivers/gpio/gpio-max3191x.c | 17 +++---
drivers/gpio/gpiolib.c | 86 +++++++++++++++--------------
drivers/gpio/gpiolib.h | 4 +-
drivers/i2c/muxes/i2c-mux-gpio.c | 3 +-
drivers/mmc/core/pwrseq_simple.c | 13 ++---
drivers/mux/gpio.c | 4 +-
drivers/net/phy/mdio-mux-gpio.c | 3 +-
drivers/pcmcia/soc_common.c | 11 ++--
drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++---
drivers/staging/iio/adc/ad7606.c | 9 +--
drivers/tty/serial/serial_mctrl_gpio.c | 7 ++-
include/linux/gpio/consumer.h | 18 +++---
15 files changed, 138 insertions(+), 147 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index aa03f389d41d..ed68042ddccf 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs::
int gpiod_get_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
The array can be an arbitrary set of GPIOs. The functions will try to access
GPIOs belonging to the same bank or chip simultaneously if supported by the
@@ -356,8 +356,8 @@ accessed sequentially.
The functions take three arguments:
* array_size - the number of array elements
* desc_array - an array of GPIO descriptors
- * value_array - an array to store the GPIOs' values (get) or
- an array of values to assign to the GPIOs (set)
+ * value_bitmap - a bitmap to store the GPIOs' values (get) or
+ a bitmap of values to assign to the GPIOs (set)
The descriptor array can be obtained using the gpiod_get_array() function
or one of its variants. If the group of descriptors returned by that function
@@ -366,7 +366,7 @@ the struct gpio_descs returned by gpiod_get_array()::
struct gpio_descs *my_gpio_descs = gpiod_get_array(...);
gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc,
- my_gpio_values);
+ my_gpio_value_bitmap);
It is also possible to access a completely arbitrary array of descriptors. The
descriptors may be obtained using any combination of gpiod_get() and
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index f1a42f0f1ded..d340473aa142 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -62,20 +62,19 @@ static void hd44780_strobe_gpio(struct hd44780 *hd)
/* write to an LCD panel register in 8 bit GPIO mode */
static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
{
- int values[10]; /* for DATA[0-7], RS, RW */
- unsigned int i, n;
+ unsigned long value_bitmap[1]; /* for DATA[0-7], RS, RW */
+ unsigned int n;
- for (i = 0; i < 8; i++)
- values[PIN_DATA0 + i] = !!(val & BIT(i));
- values[PIN_CTRL_RS] = rs;
+ value_bitmap[0] = val;
+ __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
n = 9;
if (hd->pins[PIN_CTRL_RW]) {
- values[PIN_CTRL_RW] = 0;
+ __clear_bit(PIN_CTRL_RW, value_bitmap);
n++;
}
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], values);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], value_bitmap);
hd44780_strobe_gpio(hd);
}
@@ -83,32 +82,31 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
/* write to an LCD panel register in 4 bit GPIO mode */
static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
{
- int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
- unsigned int i, n;
+ /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ unsigned long value_bitmap[0];
+ unsigned int n;
/* High nibble + RS, RW */
- for (i = 4; i < 8; i++)
- values[PIN_DATA0 + i] = !!(val & BIT(i));
- values[PIN_CTRL_RS] = rs;
+ value_bitmap[0] = val;
+ __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
n = 5;
if (hd->pins[PIN_CTRL_RW]) {
- values[PIN_CTRL_RW] = 0;
+ __clear_bit(PIN_CTRL_RW, value_bitmap);
n++;
}
+ value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
- &values[PIN_DATA4]);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
hd44780_strobe_gpio(hd);
/* Low nibble */
- for (i = 0; i < 4; i++)
- values[PIN_DATA4 + i] = !!(val & BIT(i));
+ value_bitmap[0] &= ~((1 << PIN_DATA4) - 1);
+ value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
- &values[PIN_DATA4]);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
hd44780_strobe_gpio(hd);
}
@@ -155,23 +153,23 @@ static void hd44780_write_cmd_gpio4(struct charlcd *lcd, int cmd)
/* Send 4-bits of a command to the LCD panel in raw 4 bit GPIO mode */
static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
{
- int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ unsigned long value_bitmap[1];
struct hd44780 *hd = lcd->drvdata;
- unsigned int i, n;
+ unsigned int n;
/* Command nibble + RS, RW */
- for (i = 0; i < 4; i++)
- values[PIN_DATA4 + i] = !!(cmd & BIT(i));
- values[PIN_CTRL_RS] = 0;
+ value_bitmap[0] = cmd << PIN_DATA4;
+ __clear_bit(PIN_CTRL_RS, value_bitmap);
n = 5;
if (hd->pins[PIN_CTRL_RW]) {
- values[PIN_CTRL_RW] = 0;
+ __clear_bit(PIN_CTRL_RW, value_bitmap);
n++;
}
+ value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
/* Present the data to the port */
- gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
- &values[PIN_DATA4]);
+ gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
hd44780_strobe_gpio(hd);
}
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index 073fd9011154..ce6c1e89236d 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -110,13 +110,9 @@ static void ts_nbus_set_direction(struct ts_nbus *ts_nbus, int direction)
*/
static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
{
- int i;
- int values[8];
-
- for (i = 0; i < 8; i++)
- values[i] = 0;
+ unsigned long value_bitmap[1] = { 0, };
- gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, values);
+ gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, value_bitmap);
gpiod_set_value_cansleep(ts_nbus->csn, 0);
gpiod_set_value_cansleep(ts_nbus->strobe, 0);
gpiod_set_value_cansleep(ts_nbus->ale, 0);
@@ -157,16 +153,9 @@ static int ts_nbus_read_byte(struct ts_nbus *ts_nbus, u8 *val)
static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
{
struct gpio_descs *gpios = ts_nbus->data;
- int i;
- int values[8];
-
- for (i = 0; i < 8; i++)
- if (byte & BIT(i))
- values[i] = 1;
- else
- values[i] = 0;
+ unsigned long value_bitmap[1] = { byte, };
- gpiod_set_array_value_cansleep(8, gpios->desc, values);
+ gpiod_set_array_value_cansleep(8, gpios->desc, value_bitmap);
}
/*
diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c
index b5b9cb1fda50..c4ec1c82af27 100644
--- a/drivers/gpio/gpio-max3191x.c
+++ b/drivers/gpio/gpio-max3191x.c
@@ -315,17 +315,20 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
struct gpio_desc **desc,
int value)
{
- int i, *values;
+ unsigned long *value_bitmap;
- values = kmalloc_array(ndescs, sizeof(*values), GFP_KERNEL);
- if (!values)
+ value_bitmap = kmalloc_array(BITS_TO_LONGS(ndescs),
+ sizeof(*value_bitmap), GFP_KERNEL);
+ if (!value_bitmap)
return;
- for (i = 0; i < ndescs; i++)
- values[i] = value;
+ if (value)
+ bitmap_fill(value_bitmap, ndescs);
+ else
+ bitmap_zero(value_bitmap, ndescs);
- gpiod_set_array_value_cansleep(ndescs, desc, values);
- kfree(values);
+ gpiod_set_array_value_cansleep(ndescs, desc, value_bitmap);
+ kfree(value_bitmap);
}
static struct gpio_descs *devm_gpiod_get_array_optional_count(
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8f8a1999393..f0e9ffa8cab6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -427,7 +427,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
struct linehandle_state *lh = filep->private_data;
void __user *ip = (void __user *)arg;
struct gpiohandle_data ghd;
- int vals[GPIOHANDLES_MAX];
+ unsigned long value_bitmap[BITS_TO_LONGS(GPIOHANDLES_MAX)];
int i;
if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {
@@ -436,13 +436,13 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
true,
lh->numdescs,
lh->descs,
- vals);
+ value_bitmap);
if (ret)
return ret;
memset(&ghd, 0, sizeof(ghd));
for (i = 0; i < lh->numdescs; i++)
- ghd.values[i] = vals[i];
+ ghd.values[i] = test_bit(i, value_bitmap);
if (copy_to_user(ip, &ghd, sizeof(ghd)))
return -EFAULT;
@@ -461,14 +461,14 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
/* Clamp all values to [0,1] */
for (i = 0; i < lh->numdescs; i++)
- vals[i] = !!ghd.values[i];
+ __assign_bit(i, value_bitmap, !!ghd.values[i]);
/* Reuse the array setting function */
return gpiod_set_array_value_complex(false,
true,
lh->numdescs,
lh->descs,
- vals);
+ value_bitmap);
}
return -EINVAL;
}
@@ -2784,7 +2784,7 @@ static int gpio_chip_get_multiple(struct gpio_chip *chip,
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
int i = 0;
@@ -2835,7 +2835,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
- value_array[j] = value;
+ __assign_bit(j, value_bitmap, value);
trace_gpio_value(desc_to_gpio(desc), 1, value);
}
@@ -2895,9 +2895,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
/**
* gpiod_get_raw_array_value() - read raw values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status. Return 0 in case of success,
@@ -2907,20 +2907,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
* and it will complain if the GPIO chip functions potentially sleep.
*/
int gpiod_get_raw_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(true, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
/**
* gpiod_get_array_value() - read values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitnap: bitmap to store the read values
*
* Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account. Return 0 in case of success, else an error code.
@@ -2929,12 +2930,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
* and it will complain if the GPIO chip functions potentially sleep.
*/
int gpiod_get_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(false, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value);
@@ -3027,7 +3029,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip,
int gpiod_set_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
int i = 0;
@@ -3056,7 +3058,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
do {
struct gpio_desc *desc = desc_array[i];
int hwgpio = gpio_chip_hwgpio(desc);
- int value = value_array[i];
+ int value = test_bit(i, value_bitmap);
if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
@@ -3152,9 +3154,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
/**
* gpiod_set_raw_array_value() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status.
@@ -3163,20 +3165,21 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
* complain if the GPIO chip functions potentially sleep.
*/
int gpiod_set_raw_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_set_array_value_complex(true, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
/**
* gpiod_set_array_value() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account.
@@ -3185,12 +3188,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
* complain if the GPIO chip functions potentially sleep.
*/
void gpiod_set_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return;
gpiod_set_array_value_complex(false, false, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value);
@@ -3410,9 +3414,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
/**
* gpiod_get_raw_array_value_cansleep() - read raw values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status. Return 0 in case of success,
@@ -3422,21 +3426,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
*/
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(true, true, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
/**
* gpiod_get_array_value_cansleep() - read values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account. Return 0 in case of success, else an error code.
@@ -3445,13 +3449,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
*/
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(false, true, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
@@ -3493,9 +3497,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
/**
* gpiod_set_raw_array_value_cansleep() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status.
@@ -3504,13 +3508,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
*/
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_set_array_value_complex(true, true, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);
@@ -3533,9 +3537,9 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
/**
* gpiod_set_array_value_cansleep() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account.
@@ -3544,13 +3548,13 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
*/
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return;
gpiod_set_array_value_complex(false, true, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index a7e49fef73d4..11e83d2eef89 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -187,11 +187,11 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_set_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
/* This is just passed between gpiolib and devres */
struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 401308e3d036..d675e0ca2fa4 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -27,13 +27,14 @@ struct gpiomux {
static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
{
+ unsigned long value_bitmap[1] = { val, };
int i;
for (i = 0; i < mux->data.n_gpios; i++)
mux->values[i] = (val >> i) & 1;
gpiod_set_array_value_cansleep(mux->data.n_gpios,
- mux->gpios, mux->values);
+ mux->gpios, value_bitmap);
}
static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index a8b9fee4d62a..0d6e3a5be3ba 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -40,18 +40,13 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
if (!IS_ERR(reset_gpios)) {
- int i, *values;
+ unsigned long value_bitmap[1];
int nvalues = reset_gpios->ndescs;
- values = kmalloc_array(nvalues, sizeof(int), GFP_KERNEL);
- if (!values)
- return;
+ value_bitmap[0] = value;
- for (i = 0; i < nvalues; i++)
- values[i] = value;
-
- gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc, values);
- kfree(values);
+ gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
+ value_bitmap);
}
}
diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index 6fdd9316db8b..cc2d5f50472a 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -23,14 +23,14 @@ struct mux_gpio {
static int mux_gpio_set(struct mux_control *mux, int state)
{
struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
+ unsigned long value_bitmap[1] = { state, };
int i;
for (i = 0; i < mux_gpio->gpios->ndescs; i++)
mux_gpio->val[i] = (state >> i) & 1;
gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
- mux_gpio->gpios->desc,
- mux_gpio->val);
+ mux_gpio->gpios->desc, value_bitmap);
return 0;
}
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index bc90764a8b8d..8e1ec750277e 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -27,6 +27,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
void *data)
{
struct mdio_mux_gpio_state *s = data;
+ unsigned long value_bitmap[1] = { desired_child, };
unsigned int n;
if (current_child == desired_child)
@@ -36,7 +37,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
s->values[n] = (desired_child >> n) & 1;
gpiod_set_array_value_cansleep(s->gpios->ndescs, s->gpios->desc,
- s->values);
+ value_bitmap);
return 0;
}
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index c5f2344c189b..e0f89155c474 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -351,19 +351,22 @@ static int soc_common_pcmcia_config_skt(
if (ret == 0) {
struct gpio_desc *descs[2];
- int values[2], n = 0;
+ unsigned long value_bitmap[1];
+ int n = 0;
if (skt->gpio_reset) {
descs[n] = skt->gpio_reset;
- values[n++] = !!(state->flags & SS_RESET);
+ __assign_bit(n++, value_bitmap,
+ !!(state->flags & SS_RESET));
}
if (skt->gpio_bus_enable) {
descs[n] = skt->gpio_bus_enable;
- values[n++] = !!(state->flags & SS_OUTPUT_ENA);
+ __assign_bit(n++, value_bitmap,
+ !!(state->flags & SS_OUTPUT_ENA));
}
if (n)
- gpiod_set_array_value_cansleep(n, descs, values);
+ gpiod_set_array_value_cansleep(n, descs, value_bitmap);
/*
* This really needs a better solution. The IRQ
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
index 0075fb0bef8c..b6477c3599c4 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -157,15 +157,12 @@ static const struct phy_ops gpio_usb_ops = {
*/
static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
{
- int values[PHY_MDM6600_NR_CMD_LINES];
- int i;
+ unsigned long value_bitmap[1];
- val &= (1 << PHY_MDM6600_NR_CMD_LINES) - 1;
- for (i = 0; i < PHY_MDM6600_NR_CMD_LINES; i++)
- values[i] = (val & BIT(i)) >> i;
+ value_bitmap[0] = val & ((1 << PHY_MDM6600_NR_CMD_LINES) - 1);
gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
- ddata->cmd_gpios->desc, values);
+ ddata->cmd_gpios->desc, value_bitmap);
}
/**
@@ -176,7 +173,7 @@ static void phy_mdm6600_status(struct work_struct *work)
{
struct phy_mdm6600 *ddata;
struct device *dev;
- int values[PHY_MDM6600_NR_STATUS_LINES];
+ unsigned long value_bitmap[1] = { 0, };
int error, i, val = 0;
ddata = container_of(work, struct phy_mdm6600, status_work.work);
@@ -184,14 +181,14 @@ static void phy_mdm6600_status(struct work_struct *work)
error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
ddata->status_gpios->desc,
- values);
+ value_bitmap);
if (error)
return;
for (i = 0; i < PHY_MDM6600_NR_STATUS_LINES; i++) {
- val |= values[i] << i;
+ val |= test_bit(i, value_bitmap) << i;
dev_dbg(ddata->dev, "XXX %s: i: %i values[i]: %i val: %i\n",
- __func__, i, values[i], val);
+ __func__, i, test_bit(i, value_bitmap), val);
}
ddata->status = val;
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 25b9fcd5e3a4..0eca047bc1cc 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -202,7 +202,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct ad7606_state *st = iio_priv(indio_dev);
- int values[3];
+ unsigned long value_bitmap[1];
int ret, i;
switch (mask) {
@@ -227,13 +227,10 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
if (ret < 0)
return ret;
- values[0] = (ret >> 0) & 1;
- values[1] = (ret >> 1) & 1;
- values[2] = (ret >> 2) & 1;
+ value_bitmap[0] = ret;
mutex_lock(&st->lock);
- gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
- values);
+ gpiod_set_array_value(3, st->gpio_os->desc, value_bitmap);
st->oversampling = val;
mutex_unlock(&st->lock);
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 1c06325beaca..bb8b4756d72d 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -40,7 +40,7 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
{
enum mctrl_gpio_idx i;
struct gpio_desc *desc_array[UART_GPIO_MAX];
- int value_array[UART_GPIO_MAX];
+ unsigned long value_bitmap[BITS_TO_LONGS(UART_GPIO_MAX)];
unsigned int count = 0;
if (gpios == NULL)
@@ -49,10 +49,11 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
for (i = 0; i < UART_GPIO_MAX; i++)
if (gpios->gpio[i] && mctrl_gpios_desc[i].dir_out) {
desc_array[count] = gpios->gpio[i];
- value_array[count] = !!(mctrl & mctrl_gpios_desc[i].mctrl);
+ __assign_bit(count, value_bitmap,
+ !!(mctrl & mctrl_gpios_desc[i].mctrl));
count++;
}
- gpiod_set_array_value(count, desc_array, value_array);
+ gpiod_set_array_value(count, desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(mctrl_gpio_set);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 21ddbe440030..1b21dc7b0fad 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -104,36 +104,38 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
/* Value get/set from non-sleeping context */
int gpiod_get_value(const struct gpio_desc *desc);
int gpiod_get_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array);
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap);
void gpiod_set_value(struct gpio_desc *desc, int value);
void gpiod_set_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array);
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap);
int gpiod_get_raw_value(const struct gpio_desc *desc);
int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_raw_value(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
/* Value get/set from sleeping context */
int gpiod_get_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
--
2.16.4
^ permalink raw reply related
* Re: [PATCH iproute2] iproute: make clang happy
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-08-20 23:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mahesh Bandewar, netdev
In-Reply-To: <20180820155223.0569b849@xeon-e3>
On Mon, Aug 20, 2018 at 3:52 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Mon, 20 Aug 2018 14:42:15 -0700
> Mahesh Bandewar <mahesh@bandewar.net> wrote:
>
>> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
>> index ace4b3dd738b..a524b520b276 100644
>> --- a/tc/m_ematch.c
>> +++ b/tc/m_ematch.c
>> @@ -277,6 +277,7 @@ static int flatten_tree(struct ematch *head, struct ematch *tree)
>> return count;
>> }
>>
>> +__attribute__((format(printf, 5, 6)))
>> int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>> struct ematch_util *e, char *fmt, ...)
>
> I think the printf attribute needs to go on the function prototype
> here:
> tc/m_ematch.h:extern int em_parse_error(int err, struct bstr *args, struct bstr *carg,
>
The attributes are attached to the definitions only and not prototype
declarations. Please see the definition/declaration for jsonw_printf()
in the same patch.
>
> PS: I need to take the extern of those function prototypes.
^ permalink raw reply
* Re: r8169 needs CONFIG_REALTEK_PHY
From: Marc Dionne @ 2018-08-20 23:33 UTC (permalink / raw)
To: Florian Fainelli; +Cc: hkallweit1, netdev, David Miller
In-Reply-To: <29f25302-6a60-4a44-fd65-26b5a15ff35a@gmail.com>
On Mon, Aug 20, 2018 at 5:39 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 08/20/2018 12:44 PM, Marc Dionne wrote:
>> The r8169 adapter in one of my machines was not working after updating
>> to a current kernel from the merge window, which was fixed by enabling
>> CONFIG_REALTEK_PHY.
>>
>> So in addition to "select PHYLIB", should CONFIG_R8169 not also be
>> doing "select CONFIG_REALTEK_PHY" ?
>
> This is fixed in Linus' tree now with:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bfdd19ad80f203f42f05fd32a31c678c9c524ef9
> --
> Florian
Ah thanks for the quick reply, thought I had done a fresh pull before
reporting the problem; it is indeed fixed in Linus' current tree.
Sorry for the noise,
Marc
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox