Re: [PATCH v5 1/2] memory_hotplug: Free pages as higher order
On Thu, 11 Oct 2018 09:55:03 +0200 Michal Hocko wrote:
> > > > > This is now not called anymore, although the xen/hv variants still do
> > > > > it. The function seems empty these days, maybe remove it as a followup
> > > > > cleanup?
> > > > >
> > > > > > - __online_page_increment_counters(page);
> > > > > > - __online_page_free(page);
> > > > > > + __free_pages_core(page, order);
> > > > > > + totalram_pages += (1UL << order);
> > > > > > +#ifdef CONFIG_HIGHMEM
> > > > > > + if (PageHighMem(page))
> > > > > > + totalhigh_pages += (1UL << order);
> > > > > > +#endif
> > > > >
> > > > > __online_page_increment_counters() would have used
> > > > > adjust_managed_page_count() which would do the changes under
> > > > > managed_page_count_lock. Are we safe without the lock? If yes, there
> > > > > should perhaps be a comment explaining why.
> > > >
> > > > Looks unsafe without managed_page_count_lock.
> > >
> > > Why does it matter actually? We cannot online/offline memory in
> > > parallel. This is not the case for the boot where we initialize memory
> > > in parallel on multiple nodes. So this seems to be safe currently unless
> > > I am missing something. A comment explaining that would be helpful
> > > though.
> >
> > Other main callers of adjust_manage_page_count(),
> >
> > static inline void free_reserved_page(struct page *page)
> > {
> > __free_reserved_page(page);
> > adjust_managed_page_count(page, 1);
> > }
> >
> > static inline void mark_page_reserved(struct page *page)
> > {
> > SetPageReserved(page);
> > adjust_managed_page_count(page, -1);
> > }
> >
> > Won't they race with memory hotplug?
> >
> > Few more,
> > ./drivers/xen/balloon.c:519:adjust_managed_page_count(page, -1);
> > ./drivers/virtio/virtio_balloon.c:175: adjust_managed_page_count(page, -1);
> > ./drivers/virtio/virtio_balloon.c:196: adjust_managed_page_count(page, 1);
> > ./mm/hugetlb.c:2158:adjust_managed_page_count(page, 1 <<
> > h->order);
>
> They can, and I have missed those.
So this patch needs more work, yes?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/2] memory_hotplug: Free pages as higher order
On Mon, 05 Nov 2018 15:12:27 +0530 Arun KS wrote: > On 2018-10-22 16:03, Arun KS wrote: > > On 2018-10-19 13:37, Michal Hocko wrote: > >> On Thu 18-10-18 19:18:25, Andrew Morton wrote: > >> [...] > >>> So this patch needs more work, yes? > >> > >> Yes, I've talked to Arun (he is offline until next week) offlist and > >> he > >> will play with this some more. > > > > Converted totalhigh_pages, totalram_pages and zone->managed_page to > > atomic and tested hot add. Latency is not effected with this change. > > Will send out a separate patch on top of this one. > Hello Andrew/Michal, > > Will this be going in subsequent -rcs? I thought were awaiting a new version? "Will send out a separate patch on top of this one"? I do think a resend would be useful, please. Ensure the changelog is updated to capture the above info and any other worthy issues which arose during review. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2] binder: ipc namespace support for android binder
On Mon, 29 Oct 2018 06:18:11 + chouryzhou(周威) wrote: > We are working for running android in container, but we found that binder is > not isolated by ipc namespace. Since binder is a form of IPC and therefore > should > be tied to ipc namespace. With this patch, we can run more than one android > container on one host. > This patch move "binder_procs" and "binder_context" into ipc_namespace, > driver will find the context from it when opening. Althought statistics in > debugfs > remain global. > > ... > > --- a/ipc/namespace.c > +++ b/ipc/namespace.c > @@ -56,6 +56,9 @@ static struct ipc_namespace *create_ipc_ns(struct > user_namespace *user_ns, > ns->ucounts = ucounts; > > err = mq_init_ns(ns); > + if (err) > + goto fail_put; > + err = binder_init_ns(ns); > if (err) > goto fail_put; > Don't we need an mq_put_mnt() if binder_init_ns() fails? free_ipc_ns() seems to have forgotten about that too. In which case it must be madly leaking mounts so probably I'm wrong. Confused. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2] binder: ipc namespace support for android binder
On Wed, 7 Nov 2018 01:48:12 + chouryzhou(周威) wrote: > > > --- a/ipc/namespace.c > > > +++ b/ipc/namespace.c > > > @@ -56,6 +56,9 @@ static struct ipc_namespace *create_ipc_ns(struct > > user_namespace *user_ns, > > > ns->ucounts = ucounts; > > > > > > err = mq_init_ns(ns); > > > + if (err) > > > + goto fail_put; > > > + err = binder_init_ns(ns); > > > if (err) > > > goto fail_put; > > > > > > > Don't we need an mq_put_mnt() if binder_init_ns() fails? > > > > free_ipc_ns() seems to have forgotten about that too. In which case it > > must be madly leaking mounts so probably I'm wrong. Confused. > > > > mq_init_ns will do clean job if it failed, and as do binder_init_ns. My point is that if mq_init_ns() succeeds and binder_init_ns() fails, we don't undo the effects of mq_init_ns()? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4] binder: ipc namespace support for android binder
On Mon, 12 Nov 2018 09:37:51 + chouryzhou(周威) wrote: > Currently android's binder is not isolated by ipc namespace. Since binder > is a form of IPC and therefore should be tied to ipc namespace. With this > patch, we can run multiple instances of android container on one host. > > This patch move "binder_procs" and "binder_context" into ipc_namespace, > driver will find the context from it when opening. For debugfs, binder_proc > is namespace-aware, but not for binder dead nodes, binder_stats and > binder_transaction_log_entry (we added ipc inum to trace it). > > ... > > drivers/android/binder.c | 133 > -- > include/linux/ipc_namespace.h | 15 + > ipc/namespace.c | 10 +++- > 3 files changed, 125 insertions(+), 33 deletions(-) Well, it's mainly an android patch so I suggest this be taken via the android tree. Acked-by: Andrew Morton ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/8] mm/kdump: allow to exclude pages that are logically offline
On Wed, 27 Feb 2019 13:32:14 +0800 Dave Young wrote: > This series have been in -next for some days, could we get this in > mainline? It's been in -next for two months? > Andrew, do you have plan about them, maybe next release? They're all reviewed except for "xen/balloon: mark inflated pages PG_offline". (https://ozlabs.org/~akpm/mmotm/broken-out/xen-balloon-mark-inflated-pages-pg_offline.patch). Yes, I plan on sending these to Linus during the merge window for 5.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] android: binder: Drop lru lock in isolate callback
On Thu, 14 Sep 2017 14:22:25 -0400 Sherry Yang wrote:
> Drop the global lru lock in isolate callback
> before calling zap_page_range which calls
> cond_resched, and re-acquire the global lru
> lock before returning. Also change return
> code to LRU_REMOVED_RETRY.
>
> Use mmput_async when fail to acquire mmap sem
> in an atomic context.
>
> Fix "BUG: sleeping function called from invalid context"
> errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
>
> Also restore mmput_async, which was initially introduced in
> ec8d7c14e ("mm, oom_reaper: do not mmput synchronously from
> the oom reaper context"), and was removed in 212925802
> ("mm: oom: let oom_reap_task and exit_mmap run concurrently").
Ho hum, OK. It's a bit sad to bring this back for one caller but
mm.async_put_work is there and won't be going away.
The mmdrop_async stuff should be moved into fork.c (and maybe made
static) as well. It's pointless having this stuff global and inlined,
especially mmdrop_async_fn().
Something like the below, not yet tested...
From: Andrew Morton
Subject: include/linux/sched/mm.h: uninline mmdrop_async(), etc
mmdrop_async() is only used in fork.c. Move that and its support functions
into fork.c, uninline it all.
Signed-off-by: Andrew Morton
---
include/linux/sched/mm.h | 24 +--
kernel/fork.c| 58 +
2 files changed, 42 insertions(+), 40 deletions(-)
diff -puN include/linux/sched/mm.h~a include/linux/sched/mm.h
--- a/include/linux/sched/mm.h~a
+++ a/include/linux/sched/mm.h
@@ -10,7 +10,7 @@
/*
* Routines for handling mm_structs
*/
-extern struct mm_struct * mm_alloc(void);
+extern struct mm_struct *mm_alloc(void);
/**
* mmgrab() - Pin a &struct mm_struct.
@@ -34,27 +34,7 @@ static inline void mmgrab(struct mm_stru
atomic_inc(&mm->mm_count);
}
-/* mmdrop drops the mm and the page tables */
-extern void __mmdrop(struct mm_struct *);
-static inline void mmdrop(struct mm_struct *mm)
-{
- if (unlikely(atomic_dec_and_test(&mm->mm_count)))
- __mmdrop(mm);
-}
-
-static inline void mmdrop_async_fn(struct work_struct *work)
-{
- struct mm_struct *mm = container_of(work, struct mm_struct,
async_put_work);
- __mmdrop(mm);
-}
-
-static inline void mmdrop_async(struct mm_struct *mm)
-{
- if (unlikely(atomic_dec_and_test(&mm->mm_count))) {
- INIT_WORK(&mm->async_put_work, mmdrop_async_fn);
- schedule_work(&mm->async_put_work);
- }
-}
+extern void mmdrop(struct mm_struct *mm);
/**
* mmget() - Pin the address space associated with a &struct mm_struct.
diff -puN kernel/fork.c~a kernel/fork.c
--- a/kernel/fork.c~a
+++ a/kernel/fork.c
@@ -386,6 +386,46 @@ void free_task(struct task_struct *tsk)
}
EXPORT_SYMBOL(free_task);
+/*
+ * Called when the last reference to the mm
+ * is dropped: either by a lazy thread or by
+ * mmput. Free the page directory and the mm.
+ */
+static void __mmdrop(struct mm_struct *mm)
+{
+ BUG_ON(mm == &init_mm);
+ mm_free_pgd(mm);
+ destroy_context(mm);
+ hmm_mm_destroy(mm);
+ mmu_notifier_mm_destroy(mm);
+ check_mm(mm);
+ put_user_ns(mm->user_ns);
+ free_mm(mm);
+}
+
+void mmdrop(struct mm_struct *mm)
+{
+ if (unlikely(atomic_dec_and_test(&mm->mm_count)))
+ __mmdrop(mm);
+}
+EXPORT_SYMBOL_GPL(mmdrop)
+
+static void mmdrop_async_fn(struct work_struct *work)
+{
+ struct mm_struct *mm;
+
+ mm = container_of(work, struct mm_struct, async_put_work);
+ __mmdrop(mm);
+}
+
+static void mmdrop_async(struct mm_struct *mm)
+{
+ if (unlikely(atomic_dec_and_test(&mm->mm_count))) {
+ INIT_WORK(&mm->async_put_work, mmdrop_async_fn);
+ schedule_work(&mm->async_put_work);
+ }
+}
+
static inline void free_signal_struct(struct signal_struct *sig)
{
taskstats_tgid_free(sig);
@@ -895,24 +935,6 @@ struct mm_struct *mm_alloc(void)
return mm_init(mm, current, current_user_ns());
}
-/*
- * Called when the last reference to the mm
- * is dropped: either by a lazy thread or by
- * mmput. Free the page directory and the mm.
- */
-void __mmdrop(struct mm_struct *mm)
-{
- BUG_ON(mm == &init_mm);
- mm_free_pgd(mm);
- destroy_context(mm);
- hmm_mm_destroy(mm);
- mmu_notifier_mm_destroy(mm);
- check_mm(mm);
- put_user_ns(mm->user_ns);
- free_mm(mm);
-}
-EXPORT_SYMBOL_GPL(__mmdrop);
-
static inline void __mmput(struct mm_struct *mm)
{
VM_BUG_ON(atomic_read(&mm->mm_users));
_
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 4/4] kexec_file: Load kernel at top of system RAM if required
On Wed, 18 Jul 2018 10:49:44 +0800 Baoquan He wrote: > For kexec_file loading, if kexec_buf.top_down is 'true', the memory which > is used to load kernel/initrd/purgatory is supposed to be allocated from > top to down. This is what we have been doing all along in the old kexec > loading interface and the kexec loading is still default setting in some > distributions. However, the current kexec_file loading interface doesn't > do like this. The function arch_kexec_walk_mem() it calls ignores checking > kexec_buf.top_down, but calls walk_system_ram_res() directly to go through > all resources of System RAM from bottom to up, to try to find memory region > which can contain the specific kexec buffer, then call > locate_mem_hole_callback() > to allocate memory in that found memory region from top to down. This brings > confusion especially when KASLR is widely supported , users have to make clear > why kexec/kdump kernel loading position is different between these two > interfaces in order to exclude unnecessary noises. Hence these two interfaces > need be unified on behaviour. As far as I can tell, the above is the whole reason for the patchset, yes? To avoid confusing users. Is that sufficient? Can we instead simplify their lives by providing better documentation or informative printks or better Kconfig text, etc? And who *are* the people who are performing this configuration? Random system administrators? Linux distro engineers? If the latter then they presumably aren't easily confused! In other words, I'm trying to understand how much benefit this patchset will provide to our users as a whole. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 4/4] kexec_file: Load kernel at top of system RAM if required
On Thu, 19 Jul 2018 23:17:53 +0800 Baoquan He wrote: > Hi Andrew, > > On 07/18/18 at 03:33pm, Andrew Morton wrote: > > On Wed, 18 Jul 2018 10:49:44 +0800 Baoquan He wrote: > > > > > For kexec_file loading, if kexec_buf.top_down is 'true', the memory which > > > is used to load kernel/initrd/purgatory is supposed to be allocated from > > > top to down. This is what we have been doing all along in the old kexec > > > loading interface and the kexec loading is still default setting in some > > > distributions. However, the current kexec_file loading interface doesn't > > > do like this. The function arch_kexec_walk_mem() it calls ignores checking > > > kexec_buf.top_down, but calls walk_system_ram_res() directly to go through > > > all resources of System RAM from bottom to up, to try to find memory > > > region > > > which can contain the specific kexec buffer, then call > > > locate_mem_hole_callback() > > > to allocate memory in that found memory region from top to down. This > > > brings > > > confusion especially when KASLR is widely supported , users have to make > > > clear > > > why kexec/kdump kernel loading position is different between these two > > > interfaces in order to exclude unnecessary noises. Hence these two > > > interfaces > > > need be unified on behaviour. > > > > As far as I can tell, the above is the whole reason for the patchset, > > yes? To avoid confusing users. > > > In fact, it's not just trying to avoid confusing users. Kexec loading > and kexec_file loading are just do the same thing in essence. Just we > need do kernel image verification on uefi system, have to port kexec > loading code to kernel. > > Kexec has been a formal feature in our distro, and customers owning > those kind of very large machine can make use of this feature to speed > up the reboot process. On uefi machine, the kexec_file loading will > search place to put kernel under 4G from top to down. As we know, the > 1st 4G space is DMA32 ZONE, dma, pci mmcfg, bios etc all try to consume > it. It may have possibility to not be able to find a usable space for > kernel/initrd. From the top down of the whole memory space, we don't > have this worry. > > And at the first post, I just posted below with AKASHI's > walk_system_ram_res_rev() version. Later you suggested to use > list_head to link child sibling of resource, see what the code change > looks like. > http://lkml.kernel.org/r/20180322033722.9279-1-...@redhat.com > > Then I posted v2 > http://lkml.kernel.org/r/20180408024724.16812-1-...@redhat.com > Rob Herring mentioned that other components which has this tree struct > have planned to do the same thing, replacing the singly linked list with > list_head to link resource child sibling. Just quote Rob's words as > below. I think this could be another reason. > > ~ From Rob > The DT struct device_node also has the same tree structure with > parent, child, sibling pointers and converting to list_head had been > on the todo list for a while. ACPI also has some tree walking > functions (drivers/acpi/acpica/pstree.c). Perhaps there should be a > common tree struct and helpers defined either on top of list_head or a > ~ > new struct if that saves some size. Please let's get all this into the changelogs? > > > > Is that sufficient? Can we instead simplify their lives by providing > > better documentation or informative printks or better Kconfig text, > > etc? > > > > And who *are* the people who are performing this configuration? Random > > system administrators? Linux distro engineers? If the latter then > > they presumably aren't easily confused! > > Kexec was invented for kernel developer to speed up their kernel > rebooting. Now high end sever admin, kernel developer and QE are also > keen to use it to reboot large box for faster feature testing, bug > debugging. Kernel dev could know this well, about kernel loading > position, admin or QE might not be aware of it very well. > > > > > In other words, I'm trying to understand how much benefit this patchset > > will provide to our users as a whole. > > Understood. The list_head replacing patch truly involes too many code > changes, it's risky. I am willing to try any idea from reviewers, won't > persuit they have to be accepted finally. If don't have a try, we don't > know what it looks like, and what impact it may have. I am fine to take > AKASHI's simple version of walk_system_ram_res_rev() to lower risk, even > though it could be a little bit low efficient. The larger patch produces a better result. We can handle it ;) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances
On Tue, 24 Apr 2018 16:23:04 +0200 Christoph Hellwig wrote: > On Thu, Apr 19, 2018 at 09:57:50PM +0300, Alexey Dobriyan wrote: > > > git://git.infradead.org/users/hch/misc.git proc_create > > > > > > I want to ask if it is time to start using poorman function overloading > > with _b_c_e(). There are millions of allocation functions for example, > > all slightly difference, and people will add more. Seeing /proc interfaces > > doubled like this is painful. > > Function overloading is totally unacceptable. > > And I very much disagree with a tradeoff that keeps 5000 lines of > code vs a few new helpers. OK, the curiosity and suspense are killing me. What the heck is "function overloading with _b_c_e()"? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration
On Thu, 06 Mar 2014 15:28:40 -0800 Joe Perches wrote:
> Networking prefers this style, so warn when it's not used.
>
> Networking uses:
>
> void foo(int bar)
> {
> int baz;
>
> code...
> }
>
> not
>
> void foo(int bar)
> {
> int baz;
> code...
> }
>
> There are a limited number of false positives when using
> macros to declare variables like:
>
> WARNING: networking uses a blank line after declarations
> #330: FILE: net/ipv4/inet_hashtables.c:330:
> + int dif = sk->sk_bound_dev_if;
> + INET_ADDR_COOKIE(acookie, saddr, daddr)
um wait wut wot.
*All* kernel code uses blank line between end-of-locals and
start-of-code. Or if it doesn't it should, thwap.
Why are we special-casing net/?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration
On Thu, 06 Mar 2014 15:42:30 -0800 Joe Perches wrote:
> On Thu, 2014-03-06 at 15:35 -0800, Andrew Morton wrote:
> > On Thu, 06 Mar 2014 15:28:40 -0800 Joe Perches wrote:
> > > Networking prefers this style, so warn when it's not used.
> > > void foo(int bar)
> > > {
> > > int baz;
> > >
> > > code...
> > > }
> > >
> > > not
> > >
> > > void foo(int bar)
> > > {
> > > int baz;
> > > code...
> > > }
> > >
> > > There are a limited number of false positives when using
> > > macros to declare variables like:
> > >
> > > WARNING: networking uses a blank line after declarations
> > > #330: FILE: net/ipv4/inet_hashtables.c:330:
> > > + int dif = sk->sk_bound_dev_if;
> > > + INET_ADDR_COOKIE(acookie, saddr, daddr)
> >
> > um wait wut wot.
> >
> > *All* kernel code uses blank line between end-of-locals and
> > start-of-code. Or if it doesn't it should, thwap.
> > Why are we special-casing net/?
>
> It's easy enough to remove the path test, but it's
> not in CodingStyle and David seems to be the one
> that makes the effort to correct people about it.
>
> I don't care one way or another.
>
> I'm just trying to get fewer rejections for people
> that use checkpatch.
mutter.
OK, let's do this for now. Could you please cook up a followon patch
which makes this kernel-wide? I'll play with that for a while then I'll
decide how much I feel like irritating people.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/3] memstick: Add realtek USB memstick host driver
On Wed, 12 Feb 2014 18:00:38 +0800 wrote:
> From: Roger Tseng
>
> Realtek USB memstick host driver provides memstick host support based on the
> Realtek USB card reader MFD driver.
>
> ...
>
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct rtsx_usb_ms {
> + struct platform_device *pdev;
> + struct rtsx_ucr *ucr;
> + struct memstick_host*msh;
> + struct memstick_request *req;
> +
> + struct mutexhost_mutex;
Should have included mutex.h.
> + struct work_struct handle_req;
> +
> + struct task_struct *detect_ms;
sched.h.
> + struct completion detect_ms_exit;
completion.h.
> + u8 ssc_depth;
> + unsigned intclock;
> + int power_mode;
> + unsigned char ifmode;
> + booleject;
> +};
> +
>
> ...
>
> +
> +#else
> +
> +#define ms_print_debug_regs(host)
static void ms_print_debug_regs(struct rtsx_usb_ms *host)
{
}
It is good to have the same signature for either case. And doing it in
C provides typechecking and can avoid variable-unused warnings.
> +#endif
>
> ...
>
> +static int ms_power_on(struct rtsx_usb_ms *host)
> +{
> + struct rtsx_ucr *ucr = host->ucr;
> + int err;
> +
> + dev_dbg(ms_dev(host), "%s\n", __func__);
> +
> + rtsx_usb_init_cmd(ucr);
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_SELECT, 0x07, MS_MOD_SEL);
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_SHARE_MODE,
> + CARD_SHARE_MASK, CARD_SHARE_MS);
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_CLK_EN,
> + MS_CLK_EN, MS_CLK_EN);
> + err = rtsx_usb_send_cmd(ucr, MODE_C, 100);
> + if (err < 0)
> + return err;
> +
> + if (CHECK_PKG(ucr, LQFP48))
> + err = ms_pull_ctl_enable_lqfp48(ucr);
> + else
> + err = ms_pull_ctl_enable_qfn24(ucr);
> + if (err < 0)
> + return err;
> +
> + err = rtsx_usb_write_register(ucr, CARD_PWR_CTL,
> + POWER_MASK, PARTIAL_POWER_ON);
> + if (err)
> + return err;
> +
> + /* Wait ms power stable */
Comment is strange.
> + usleep_range(800, 1000);
> +
> + rtsx_usb_init_cmd(ucr);
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_PWR_CTL,
> + POWER_MASK, POWER_ON);
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, CARD_OE,
> + MS_OUTPUT_EN, MS_OUTPUT_EN);
> +
> + return rtsx_usb_send_cmd(ucr, MODE_C, 100);
> +}
> +
>
> ...
>
> +static int ms_transfer_data(struct rtsx_usb_ms *host, unsigned char data_dir,
> + u8 tpc, u8 cfg, struct scatterlist *sg)
> +{
> + struct rtsx_ucr *ucr = host->ucr;
> + int err;
> + unsigned int length = sg->length;
> + u16 sec_cnt = (u16)(length / 512);
> + u8 trans_mode, dma_dir, flag;
> + unsigned int pipe;
> + struct memstick_dev *card = host->msh->card;
> +
> + dev_dbg(ms_dev(host), "%s: tpc = 0x%02x, data_dir = %s, length = %d\n",
length = %u
> + __func__, tpc, (data_dir == READ) ? "READ" : "WRITE",
> + length);
> +
> + if (data_dir == READ) {
> + flag = MODE_CDIR;
> + dma_dir = DMA_DIR_FROM_CARD;
> + if (card->id.type != MEMSTICK_TYPE_PRO)
> + trans_mode = MS_TM_NORMAL_READ;
> + else
> + trans_mode = MS_TM_AUTO_READ;
> + pipe = usb_rcvbulkpipe(ucr->pusb_dev, EP_BULK_IN);
> + } else {
> + flag = MODE_CDOR;
> + dma_dir = DMA_DIR_TO_CARD;
> + if (card->id.type != MEMSTICK_TYPE_PRO)
> + trans_mode = MS_TM_NORMAL_WRITE;
> + else
> + trans_mode = MS_TM_AUTO_WRITE;
> + pipe = usb_sndbulkpipe(ucr->pusb_dev, EP_BULK_OUT);
> + }
> +
> + rtsx_usb_init_cmd(ucr);
> +
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MS_TPC, 0xFF, tpc);
> + if (card->id.type == MEMSTICK_TYPE_PRO) {
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MS_SECTOR_CNT_H,
> + 0xFF, (u8)(sec_cnt >> 8));
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MS_SECTOR_CNT_L,
> + 0xFF, (u8)sec_cnt);
> + }
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MS_TRANS_CFG, 0xFF, cfg);
> +
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MC_DMA_TC3,
> + 0xFF, (u8)(length >> 24));
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MC_DMA_TC2,
> + 0xFF, (u8)(length >> 16));
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MC_DMA_TC1,
> + 0xFF, (u8)(length >> 8));
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MC_DMA_TC0, 0xFF,
> + (u8)length);
> + rtsx_usb_add_cmd(ucr, WRITE_REG_CMD, MC_DMA_CTL,
> + 0x03 | DMA_PACK_
Re: [PATCH v4 3/3] memstick: Add realtek USB memstick host driver
On Thu, 20 Mar 2014 16:38:03 +0800 Roger wrote: > On 02/12/2014 06:00 PM, rogera...@realtek.com wrote: > > From: Roger Tseng > > > > Realtek USB memstick host driver provides memstick host support based on the > > Realtek USB card reader MFD driver. > > > > Signed-off-by: Roger Tseng > Andrew, > > Would you please Ack or comment this patch(3/3) to let the 3 patches be > merged together? I have been making the same request at the message > thread of "[PATCH v4 2/3]" since several weeks ago but got no response. > Thus I re-post here and hope I could get something. > Looks OK to me - I had a few minor quibbles. Please proceed in that way. It would be nice if Alex and/or Maxim had time to look through the code. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] quiet checkpatch style recommendation about no spaces around bitfield :
On Mon, 31 Mar 2014 08:31:38 -0700 Joe Perches wrote:
> > > @@ -143,13 +143,13 @@ union cvmx_usbcx_gahbcfg {
> > >* * 1'b1: Unmask the interrupt assertion to the application.
> > >*/
> > > struct cvmx_usbcx_gahbcfg_s {
> > > - uint32_t reserved_9_31 : 23;
> > > - uint32_t ptxfemplvl : 1;
> > > - uint32_t nptxfemplvl: 1;
> > > - uint32_t reserved_6_6 : 1;
> > > - uint32_t dmaen : 1;
> > > - uint32_t hbstlen: 4;
> > > - uint32_t glblintrmsk: 1;
> > > + uint32_t reserved_9_31:23;
> > > + uint32_t ptxfemplvl:1;
> > > + uint32_t nptxfemplvl:1;
> > > + uint32_t reserved_6_6:1;
> > > + uint32_t dmaen:1;
> > > + uint32_t hbstlen:4;
> > > + uint32_t glblintrmsk:1;
> > > } s;
> > The warning here is:
> > ERROR: spaces prohibited around that ':' (ctx:WxW)
> > I have done a kernel wide search for these warnings
>
> Really? How?
>
> > and I think we
> > should disable this warning. It has too many false positives like this.
>
> It does seem a bit noisy to me too.
>
> Andy? Andrew?
I suspect we don't use bitfields enough for a particular style to have
emerged. Personally I think the above patch made the code harder to
read, so...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 3/3] memstick: Add realtek USB memstick host driver
On Tue, 1 Apr 2014 11:20:32 +0800 Roger wrote: > On 03/25/2014 06:44 PM, rogera...@realtek.com wrote: > > From: Roger Tseng > > > > Realtek USB memstick host driver provides memstick host support based on the > > Realtek USB card reader MFD driver. > > > > Signed-off-by: Roger Tseng > > --- > > drivers/memstick/host/Kconfig | 10 + > > drivers/memstick/host/Makefile | 1 + > > drivers/memstick/host/rtsx_usb_ms.c | 839 > > > > 3 files changed, 850 insertions(+) > > create mode 100644 drivers/memstick/host/rtsx_usb_ms.c > Hi Andrew, > > Since I'll have to send next revision(v6, to modify patch 1/3) and > you've commented v4, would you also comment or Ack the 3/3 of this one? > This should save one revision for me to make any necessary change. It looks OK to my inexpert eye. It would be better if Maxim and/or Alex could review the code. If that doesn't happen then I guess the best we can do is to go ahead and merge it. Have you worked out via which tree the patches will be merged? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] driver core: export lock_device_hotplug/unlock_device_hotplug
On Wed, 11 Feb 2015 16:44:20 +0100 Vitaly Kuznetsov wrote:
> add_memory() is supposed to be run with device_hotplug_lock grabbed, otherwise
> it can race with e.g. device_online(). Allow external modules (hv_balloon for
> now) to lock device hotplug.
>
> ...
>
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -55,11 +55,13 @@ void lock_device_hotplug(void)
> {
> mutex_lock(&device_hotplug_lock);
> }
> +EXPORT_SYMBOL_GPL(lock_device_hotplug);
>
> void unlock_device_hotplug(void)
> {
> mutex_unlock(&device_hotplug_lock);
> }
> +EXPORT_SYMBOL_GPL(unlock_device_hotplug);
>
> int lock_device_hotplug_sysfs(void)
> {
It's kinda crazy that lock_device_hotplug_sysfs() didn't get any
documentation. I suggest adding this while you're in there:
--- a/drivers/base/core.c~a
+++ a/drivers/base/core.c
@@ -61,6 +61,9 @@ void unlock_device_hotplug(void)
mutex_unlock(&device_hotplug_lock);
}
+/*
+ * "git show 5e33bc4165f3ed" for details
+ */
int lock_device_hotplug_sysfs(void)
{
if (mutex_trylock(&device_hotplug_lock))
which is a bit lazy but whatev.
I'll assume that Greg (or Rafael?) will be processing this patchset.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RESEND 0/3] memory_hotplug: hyperv: fix deadlock between memory adding and onlining
On Thu, 12 Feb 2015 23:43:17 +0100 "Rafael J. Wysocki" wrote: > On Thursday, February 12, 2015 10:10:30 PM KY Srinivasan wrote: > > [cut] yay! > > > > > > > > > > > > This issue was first discovered by Andy Whitcroft: > > > > > > https://lkml.org/lkml/2014/3/14/451 > > > > > > I had sent patches based on Andy's analysis that did not affect > > > > > > the users of the kernel hot-add memory APIs: > > > > > > https://lkml.org/lkml/2014/12/2/662 > > > > > > > > > > > > This patch puts the burden where it needs to be and can address > > > > > > the issue > > > > > for all clients. > > > > > > > > > > That seems to mean that this series is not needed. Is that correct? > > > > > > > > This patch was never committed upstream and so the issue still is there. > > > > > > Well, I'm not sure what to do now to be honest. > > > > > > Is this series regarded as the right way to address the problem that > > > everybody is comfortable with? Or is it still under discussion? > > > > We need to solve this problem and that is not under discussion. I also > > believe this problem > > needs to be solved in a way that addresses the problem where it belongs - > > not in the users of > > the hot_add API. Both my solution and the one proposed by David > > https://lkml.org/lkml/2015/2/12/57 > > address this issue. You can select either patch and check it in. I just > > want the issue addressed and I am not > > married to the solution I proposed. > > OK, thanks! > > So having looked at both your patch and the David's one I think that > the Andrew's tree is appropriate for any of them. > > Andrew? OK, I'll wake up and take a look. Hopefully as 3.21 material but I need to to back and reread everything. Is it more urgent than that? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] memstick: rtsx: fix ms card data transfer bug
On Wed, 30 Oct 2013 14:40:16 +0800 wrote: > unlike mspro card, ms card use normal read/write mode for DMA > data transfer. What are the user-visible effects of this bug? Please always include this information when fixing bugs so that others can decide whether they (or their customers) need the patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] memstick: rtsx: fix ms card data transfer bug
On Wed, 6 Nov 2013 09:14:59 +0800 micky wrote:
> On 11/06/2013 05:10 AM, Andrew Morton wrote:
> > On Wed, 30 Oct 2013 14:40:16 +0800 wrote:
> >
> >> unlike mspro card, ms card use normal read/write mode for DMA
> >> data transfer.
> > What are the user-visible effects of this bug?
> >
> > Please always include this information when fixing bugs so that others
> > can decide whether they (or their customers) need the patch.
> >
>
(top-posting repaired - please don't top-post!)
> MS card can not use auto read/write mode, so it will fail at
> initialize and long data transfer. This patch is used to add
> support for ms card.
>
> Shall I re-send this patch to add more info?
That's OK - I updated the changelog in-place and added cc:stable so it
gets backported. But then I dropped the patch ;)
>From this info I assume that use of ms cards is very rare, otherwise
people would have complained. What is the difference between an "ms
card" and an "mspro card"? How common are each type and what is their
availability?
ms_transfer_data() and mspro_transfer_data() are very similar. I think
it would be more maintainable if they were integrated into a single
function?
trans_done and timeleft could be made local to the code block where
they used. This would be neater and more maintainable.
This code is troublesome:
: if (pcr->trans_result == TRANS_NOT_READY) {
: init_completion(&trans_done);
: timeleft = wait_for_completion_interruptible_timeout(
: &trans_done, 1000);
: if (timeleft < 0) {
: dev_dbg(ms_dev(host),
: "%s: timeout wait for ok interrupt.\n",
: __func__);
: return -ETIMEDOUT;
: }
: }
- Why does it exist? Needs a comment explaining what it is trying to
achieve.
- It should use DECLARE_COMPLETION_ONSTACK() for trans_done
- It uses wait_for_completion() but nothing ever calls complete() on
the object! That's just bizarre and more appropriate primitives
should be used.
- The debug message is hard to understand and appears to be wrong.
Should be "interrupt received while waiting for ".
- The code appears to be terminating a kernel IO transaction when the
user hits ^C. That's just not viable - the ^C could have been
entered for other reasons and the IO will complete just fine. Also
no -EINTR is returned callers don't appear to be set up to handle it.
So shudder. I'll drop the patch. Please explain very carefully what
you're trying to achieve here and perhaps we can suggest a suitable
implementation approach.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch: Improve missing blank line after declarations test
On Mon, 05 May 2014 13:12:16 -0700 Joe Perches wrote:
> A couple more modifications to the declarations tests.
>
> o Declarations can also be bitfields so exclude things with a colon
> o Make sure the current and previous lines are indented the same
> to avoid matching some macro where a struct type is passed on
> the previous line like:
>
> next = list_entry(buffer->entry.next,
> struct binder_buffer, entry);
> if (buffer_start_page(next) == buffer_end_page(buffer))
So
checkpatch-always-warn-on-missing-blank-line-after-variable-declaration-block.patch
is stuck in -mm while I evaluate its effects. Thus far that evaluation
has been "super non-intrusive", because the patch doesn't actually
do anything.
--- a/fs/open.c~a
+++ a/fs/open.c
@@ -39,6 +39,7 @@ int do_truncate(struct dentry *dentry, l
{
int ret;
struct iattr newattrs;
+ wibble();
/* Not pretty: "inode->i_size" shouldn't really be signed. But it is. */
if (length < 0)
@@ -67,6 +68,7 @@ long vfs_truncate(struct path *path, lof
{
struct inode *inode;
long error;
+ wobble();
inode = path->dentry->d_inode;
I add --strict and it still doesn't warn. What did I do wrong this time?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch: Improve missing blank line after declarations test
On Mon, 05 May 2014 15:35:43 -0700 Joe Perches wrote:
> > @@ -67,6 +68,7 @@ long vfs_truncate(struct path *path, lof
> > {
> > struct inode *inode;
> > long error;
> > + wobble();
> >
> > inode = path->dentry->d_inode;
>
> Patch content can be a bit odd when lines are
> both added and deleted so checkpatch bleats
> only when both lines are added.
>
> + int foo;
> + wibble();
>
> generates a complaint.
>
> int foo;
> + wibble_wobble();
>
> does not.
Oh, OK.
I have seen no instances of this warning since adding the patch. So I
guess it's safe to merge but perhaps insufficiently aggressive. Or
maybe people are being well-behaved.
Oh well, I'll keep an eye out.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] list: Fix order of arguments for hlist_add_after(_rcu)
On Fri, 6 Jun 2014 10:22:51 -0700 "Paul E. McKenney" wrote: > On Fri, Jun 06, 2014 at 03:56:52PM +, David Laight wrote: > > From: Behalf Of Ken Helias > > > All other add functions for lists have the new item as first argument and > > > the > > > position where it is added as second argument. This was changed for no > > > good > > > reason in this function and makes using it unnecessary confusing. > > > > > > Also the naming of the arguments in hlist_add_after was confusing. It was > > > changed to use the same names as hlist_add_after_rcu. > > ... > > > -static inline void hlist_add_after_rcu(struct hlist_node *prev, > > > -struct hlist_node *n) > > > +static inline void hlist_add_after_rcu(struct hlist_node *n, > > > +struct hlist_node *prev) > > > > It is rather a shame that the change doesn't generate a compilation > > error for old source files. > > I am also a bit concerned by this. > yup. hlist_add_behind()? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] lib / string_helpers: clean up test suite
On Wed, 2 Jul 2014 16:20:24 +0300 Andy Shevchenko
wrote:
> This patch prepares test suite for a following update. It introduces
> test_string_check_buf() helper which checks the result and dumps an error.
>
> ...
>
> --- a/lib/test-string_helpers.c
> +++ b/lib/test-string_helpers.c
> @@ -10,6 +10,26 @@
> #include
> #include
>
> +static __init bool test_string_check_buf(const char *name, unsigned int
> flags,
> + char *in, size_t p,
> + char *out_real, size_t q_real,
> + char *out_test, size_t q_test)
> +{
> + if (q_real == q_test && !memcmp(out_test, out_real, q_test))
> + return true;
> +
> + pr_err("Test '%s' failed: flags = %u\n", name, flags);
> +
> + print_hex_dump(KERN_WARNING, "Input: ", DUMP_PREFIX_NONE, 16, 1,
> +in, p, true);
> + print_hex_dump(KERN_WARNING, "Expected: ", DUMP_PREFIX_NONE, 16, 1,
> +out_test, q_test, true);
> + print_hex_dump(KERN_WARNING, "Got: ", DUMP_PREFIX_NONE, 16, 1,
> +out_real, q_real, true);
Seems strange to mix KERN_ERR and KERN_WARNING. The code's always been
that way, but maybe it can be improved.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] lib / string_helpers: introduce string_escape_mem()
On Wed, 2 Jul 2014 16:20:25 +0300 Andy Shevchenko
wrote:
> This is almost the opposite function to string_unescape(). Nevertheless it
> handles \0 and could be used for any byte buffer.
>
> The documentation is supplied together with the function prototype.
>
> The test cases covers most of the scenarios and would be expanded later on.
>
> ...
>
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -71,4 +71,87 @@ static inline int string_unescape_any_inplace(char *buf)
> return string_unescape_any(buf, buf, 0);
> }
>
> +#define ESCAPE_SPACE 0x01
> +#define ESCAPE_SPECIAL 0x02
> +#define ESCAPE_NULL 0x04
> +#define ESCAPE_OCTAL 0x08
> +#define ESCAPE_ANY \
> + (ESCAPE_SPACE | ESCAPE_OCTAL | ESCAPE_SPECIAL | ESCAPE_NULL)
> +#define ESCAPE_NP0x10
> +#define ESCAPE_ANY_NP(ESCAPE_ANY | ESCAPE_NP)
> +#define ESCAPE_HEX 0x20
> +
> +/**
> + * string_escape_mem - quote characters in the given memory buffer
It drive me nuts when the kerneldoc is in the .h file. Who thinks of
looking there? I realise that string_unescape() already did that, but
I'd prefer that we fix string_unescape() rather than imitate it.
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
This is a lot of code! Adds nearly a kbyte. I'm surprised that
escaping a string is so verbose.
I wonder if the implementation really needs to be so comprehensive?
Would a table-driven approach be more compact?
> static int __init test_string_helpers_init(void)
> {
> unsigned int i;
> @@ -112,6 +315,16 @@ static int __init test_string_helpers_init(void)
> test_string_unescape("unescape inplace",
>get_random_int() % (UNESCAPE_ANY + 1), true);
>
> + /* Without dictionary */
> + for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> + test_string_escape("escape 0", escape0, i,
> TEST_STRING_2_DICT_0);
> +
> + /* With dictionary */
> + for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> + test_string_escape("escape 1", escape1, i,
> TEST_STRING_2_DICT_1);
> +
> + test_string_escape_nomem();
> +
> return -EINVAL;
> }
I wonder why this returns -EINVAL.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 00/11] lib: introduce string_escape_mem and %*pE specifier
On Thu, 28 Aug 2014 14:33:30 -0400 "John W. Linville" wrote: > On Wed, Aug 20, 2014 at 12:42:41PM +0300, Andy Shevchenko wrote: > > The introduced function is a kind of opposite to string_unescape. We have > > several users of such functionality each of them created custom > > implementation. > > The series contains clean up of test suite, adding new call, and switching > > few > > users to use it via %*pE specifier. > > Any objections? Do you (Andy) want me to merge this through the > wireless tree? > Who is this top-poster and what have you done with John? I was going to apply them yesterday but in [4/11] Andy said he plans on sending out a v5. (v4 actually?) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 00/11] lib: introduce string_escape_mem and %*pE specifier
On Thu, 28 Aug 2014 23:58:45 +0300 Andy Shevchenko
wrote:
> On Thu, Aug 28, 2014 at 10:08 PM, Andrew Morton
> wrote:
> > On Thu, 28 Aug 2014 14:33:30 -0400 "John W. Linville"
> > wrote:
> >
> >> On Wed, Aug 20, 2014 at 12:42:41PM +0300, Andy Shevchenko wrote:
> >> > The introduced function is a kind of opposite to string_unescape. We have
> >> > several users of such functionality each of them created custom
> >> > implementation.
> >> > The series contains clean up of test suite, adding new call, and
> >> > switching few
> >> > users to use it via %*pE specifier.
> >>
> >> Any objections? Do you (Andy) want me to merge this through the
> >> wireless tree?
> >>
> >
> > Who is this top-poster and what have you done with John?
> >
> > I was going to apply them yesterday but in [4/11] Andy said he plans on
> > sending out a v5. (v4 actually?)
>
> For now (so far no more comments) it is only couple of trivia fixes
> (removing useless comments). Would you like to resend whole series or
> just fixup would be enough?
I fixed up the one Joe commented on.
--- a/lib/vsprintf.c~lib-vsprintf-add-%pe-format-specifier-fix
+++ a/lib/vsprintf.c
@@ -,12 +,11 @@ char *escaped_string(char *buf, char *en
int len;
if (spec.field_width == 0)
- /* nothing to print */
- return buf;
+ return buf; /* nothing to print */
if (ZERO_OR_NULL_PTR(addr))
- /* NULL pointer */
- return string(buf, end, NULL, spec);
+ return string(buf, end, NULL, spec);/* NULL pointer */
+
do {
switch (fmt[count++]) {
_
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC patch] checkpatch: test identifier lengths
On Fri, 16 Feb 2018 09:13:27 -0800 Joe Perches wrote:
> On Fri, 2018-02-16 at 15:55 +0300, Dan Carpenter wrote:
> > On Fri, Feb 16, 2018 at 05:06:34PM +0530, Yash Omer wrote:
> > > This patch fix line should not end with open parenthesis found by
> > > checkpatch.plscript.
> > >
> > > Signed-off-by: Yash Omer
> > > ---
> > > drivers/staging/nvec/nvec.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> > > index 52054a528723..39fb737543b5 100644
> > > --- a/drivers/staging/nvec/nvec.c
> > > +++ b/drivers/staging/nvec/nvec.c
> > > @@ -383,8 +383,8 @@ static void nvec_request_master(struct work_struct
> > > *work)
> > > msg = list_first_entry(&nvec->tx_data, struct nvec_msg, node);
> > > spin_unlock_irqrestore(&nvec->tx_lock, flags);
> > > nvec_gpio_set_value(nvec, 0);
> > > - err = wait_for_completion_interruptible_timeout(
> > > - &nvec->ec_transfer, msecs_to_jiffies(5000));
> > > + err = wait_for_completion_interruptible_timeout
> > > + (&nvec->ec_transfer, msecs_to_jiffies(5000));
> >
> > The original code is basically fine... It's OK to ignore checkpatch in
> > this situation.
>
> Right.
Yes, I'd say that checkpatch is simply wrong here. I'd prefer that a
function call always have the opening paren hard up against the
function name. Because I often search for "foo(" to find the callsites
of foo() and I expect that some code-parsing tools do the same thing.
The "(" is the application of an operator to an identifier.
So I'd vote for simply nuking that checkpatch warning altogether.
Maybe there are other situations in which it is useful, dunno.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/2] mm, treewide: Rename kzfree() to kfree_sensitive()
On Tue, 16 Jun 2020 11:43:11 -0400 Waiman Long wrote: > As said by Linus: > > A symmetric naming is only helpful if it implies symmetries in use. > Otherwise it's actively misleading. > > In "kzalloc()", the z is meaningful and an important part of what the > caller wants. > > In "kzfree()", the z is actively detrimental, because maybe in the > future we really _might_ want to use that "memfill(0xdeadbeef)" or > something. The "zero" part of the interface isn't even _relevant_. > > The main reason that kzfree() exists is to clear sensitive information > that should not be leaked to other future users of the same memory > objects. > > Rename kzfree() to kfree_sensitive() to follow the example of the > recently added kvfree_sensitive() and make the intention of the API > more explicit. In addition, memzero_explicit() is used to clear the > memory to make sure that it won't get optimized away by the compiler. > > The renaming is done by using the command sequence: > > git grep -w --name-only kzfree |\ > xargs sed -i 's/\bkzfree\b/kfree_sensitive/' > > followed by some editing of the kfree_sensitive() kerneldoc and adding > a kzfree backward compatibility macro in slab.h. > > ... > > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -186,10 +186,12 @@ void memcg_deactivate_kmem_caches(struct mem_cgroup *, > struct mem_cgroup *); > */ > void * __must_check krealloc(const void *, size_t, gfp_t); > void kfree(const void *); > -void kzfree(const void *); > +void kfree_sensitive(const void *); > size_t __ksize(const void *); > size_t ksize(const void *); > > +#define kzfree(x)kfree_sensitive(x) /* For backward compatibility */ > + What was the thinking here? Is this really necessary? I suppose we could keep this around for a while to ease migration. But not for too long, please. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
