Re: syscall: introduce sendfd() syscall (v.2)
On Sat, Dec 6, 2014 at 12:22 AM, One Thousand Gnomes wrote: > >> 2.a. If task A has sufficient capabilities to send signals to task B, then >> task A is already in position to do anything it wants with task B, including >> killing it outright. > > Not entirely true. > > - We have securirty models like SELinux > - We have namespaces and being able to send an fd between namespaces is > not quite as flexible as you would make it > > I suspect therefore it needs security hooks but otherwise looks more sane > than the current AF_UNIX approach. > The best part about signal transport compared to anything in net/ is that it adheres to very straightforward and simple API contract. That is, you can tweak it here and there and still keep everything working. 1. adding an additional capability flag to selinux does not appear to be that complicated (it's got 4 capabilities related to signal handling already, fifth is not going to make much difference) 2. sending fds between namespaces may be prohibited outright; this would not be an unreasonable prohibition. A more flexible model may also be feasible, but I wonder if necessary. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] syscall: introduce sendfd() syscall (v.2)
On Sat, Dec 6, 2014 at 6:23 AM, Bastien ROUCARIES wrote: > > > See senfd recvfd in gnulib. It wirk even under solaris > What's so special about a thin wrapper around domain sockets/named fifos (solaris style)? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] syscall: introduce sendfd() syscall (v.2)
On Sat, Dec 6, 2014 at 6:23 AM, Bastien ROUCARIES roucaries.bast...@gmail.com wrote: See senfd recvfd in gnulib. It wirk even under solaris What's so special about a thin wrapper around domain sockets/named fifos (solaris style)? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: syscall: introduce sendfd() syscall (v.2)
On Sat, Dec 6, 2014 at 12:22 AM, One Thousand Gnomes gno...@lxorguk.ukuu.org.uk wrote: 2.a. If task A has sufficient capabilities to send signals to task B, then task A is already in position to do anything it wants with task B, including killing it outright. Not entirely true. - We have securirty models like SELinux - We have namespaces and being able to send an fd between namespaces is not quite as flexible as you would make it I suspect therefore it needs security hooks but otherwise looks more sane than the current AF_UNIX approach. The best part about signal transport compared to anything in net/ is that it adheres to very straightforward and simple API contract. That is, you can tweak it here and there and still keep everything working. 1. adding an additional capability flag to selinux does not appear to be that complicated (it's got 4 capabilities related to signal handling already, fifth is not going to make much difference) 2. sending fds between namespaces may be prohibited outright; this would not be an unreasonable prohibition. A more flexible model may also be feasible, but I wonder if necessary. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 9:41 PM, Richard Cochran wrote: > In any case, I find it hard to believe that the traditional method is > really so bad. The explanation of why this new way is needed boils > down to: "unix programming is so hard to get right." Surely, this can be said about any new feature proposed. Why do we need this new thing called wheel? We lived 50k years without it just fine! It all boils down to: "walking with legs is so hard to get right". :-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
syscall: introduce sendfd() syscall (v.2)
I would like to present my second attempt at file descriptor duplication over Posix.1b real-time signal transport. All the constructive points raised in the previous discussion are believed to be addressed. To this end, I would like to address some concerns raised in the preceding discussion: 1. Claim: signals as a transport would not scale Each task_struct allocated by kernel has its own signal queue, reliable, when Posix.1b signals are concerned. This queue essentially serves as per-task mail box, enabling complex applications to send signals from each thread to each thread directly, with very low overhead, and thus avoid any shared contention points outright (originating task's pid is passed along with the siginfo data, so source based dispatching is perfectly possible). Also, signals can be trivially integrated with other communication mediums, as signalfd() syscall is perfectly compatible with epoll. 2. Claim: adding new functionality to the signal transport will create new attack/DoS vectors. Nothing can be further from truth. 2.a. If task A has sufficient capabilities to send signals to task B, then task A is already in position to do anything it wants with task B, including killing it outright. 2.b. Flood attacks on signal queues are not dangerous to the system, as signal queues are relatively shallow and consume little memory even when full. Compare with infamous "recursive fd" attack against AF_UNIX fd transport , which plagues application development to this day (due to safeguards introduced to alleviate it). 2.c. Natural decoupling of signal transport from vfs internals prevents any sort of "recursive fd" attacks altogether (it is even safe to send the signalfd() fd through - this can be considered a convenient feature to replicate signal delivery masks around; of course, the receiving task will only receive its own signals through it, peeking on other task's signals will not be possible). 3. Suggestion: new file desriptors should not appear in destination processes out of the blue. 3.a. To receive the signal, process must make non-trivial preparations ( manipulate signal masks, etc), which would only happen if certain signals are expected. 3.b. In present implementation, file desriptor is only created at the destination when destination task explictly elects to receive the associated signal info with sigtimedwait/signalfd. In the absence of destination task cooperation, the only overhead on the kernel side will be a single pair of ref_count increment/decrement, that is, completely negligible. 3.c. Due to the nature of siginfo delivery, operations on file descriptor table are completely safe and indistinguishable from a normal dup() system call. I would appreciate any additional constructive criticism, as it is in my interest as well to end up with safe and simple solution. However, I would prefer the criticism to target particular technical shortcomings, and not be derived from personal preferences, if possible. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] syscall: introduce sendfd() syscall (v.2)
Present patch introduces exceptionally easy to use, low latency and low overhead mechanism for transferring file descriptors between cooperating processes: int sendfd(pid_t pid, int sig, int fd) Given a target process pid, the sendfd() will queue a real-time signal for delivery to task referenced by pid. If signal can be delivered to destination tasks and it chooses to collect the associated signal info, a new file descriptor will be created on its behalf, pointing to file originally referred by fd (the value of newly created file descriptor will be communicated as integer payload within the siginfo data). Signed-off-by: Alex Dubov --- arch/x86/syscalls/syscall_32.tbl | 2 + arch/x86/syscalls/syscall_64.tbl | 1 + include/asm-generic/siginfo.h | 1 + include/linux/syscalls.h | 1 + include/uapi/asm-generic/siginfo.h | 1 + init/Kconfig | 11 + kernel/signal.c| 89 ++ kernel/sys_ni.c| 3 ++ 8 files changed, 109 insertions(+) diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl index 9fe1b5d..e2782bd 100644 --- a/arch/x86/syscalls/syscall_32.tbl +++ b/arch/x86/syscalls/syscall_32.tbl @@ -364,3 +364,5 @@ 355i386getrandom sys_getrandom 356i386memfd_createsys_memfd_create 357i386bpf sys_bpf +358i386sendfd sys_sendfd + diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl index 281150b..4d6b55d 100644 --- a/arch/x86/syscalls/syscall_64.tbl +++ b/arch/x86/syscalls/syscall_64.tbl @@ -328,6 +328,7 @@ 319common memfd_createsys_memfd_create 320common kexec_file_load sys_kexec_file_load 321common bpf sys_bpf +322common sendfd sys_sendfd # # x32-specific system call numbers start at 512 to avoid cache impact diff --git a/include/asm-generic/siginfo.h b/include/asm-generic/siginfo.h index 3d1a3af..c8af06f 100644 --- a/include/asm-generic/siginfo.h +++ b/include/asm-generic/siginfo.h @@ -12,6 +12,7 @@ #define __SI_RT(5 << 16) #define __SI_MESGQ (6 << 16) #define __SI_SYS (7 << 16) +#define __SI_FILEP (8 << 16) #define __SI_CODE(T,N) ((T) | ((N) & 0x)) struct siginfo; diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index bda9b81..1871b72f 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -877,4 +877,5 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags, asmlinkage long sys_getrandom(char __user *buf, size_t count, unsigned int flags); asmlinkage long sys_bpf(int cmd, union bpf_attr *attr, unsigned int size); +asmlinkage long sys_sendfd(pid_t pid, int sig, int fd); #endif diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index ba5be7f..a92e38e 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -148,6 +148,7 @@ typedef struct siginfo { #define __SI_RT0 #define __SI_MESGQ 0 #define __SI_SYS 0 +#define __SI_FILEP 0 #define __SI_CODE(T,N) (N) #endif diff --git a/init/Kconfig b/init/Kconfig index 2081a4d..6a62a44 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1505,6 +1505,17 @@ config SIGNALFD If unsure, say Y. +config SENDFD + bool "Enable sendfd() system call" if EXPERT + default y + help + Enable the sendfd() system call that allows rapid duplication + of file descriptor across process boundaries. The target process + will receive a duplicate file descriptor delivered with one of + Posix.1b real-time signals. + + If unsure, say Y. + config TIMERFD bool "Enable timerfd() system call" if EXPERT select ANON_INODES diff --git a/kernel/signal.c b/kernel/signal.c index 8f0876f..299ee9c 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -35,6 +35,11 @@ #include #include +#ifdef CONFIG_SENDFD +#include +#include +#endif + #define CREATE_TRACE_POINTS #include @@ -394,8 +399,15 @@ __sigqueue_alloc(int sig, struct task_struct *t, gfp_t flags, int override_rlimi static void __sigqueue_free(struct sigqueue *q) { + if (q->info.si_code == __SI_FILEP) { + fput((struct file *)q->info.si_ptr); + q->info.si_code = 0; + q->info.si_ptr = NULL; + } + if (q->flags & SIGQUEUE_PREALLOC) return; + atomic_dec(>user->sigpending); free_uid(q->user); kmem_cache_free(sigqueue_cachep, q); @@ -543,6 +555,44 @@ unblock_all_signals(void) spin_unlock_irqrestore(>sighand->siglock, flags); } +#ifdef CONFIG_SENDFD + +/* + * sendfd_copy_install can onl
[PATCH] syscall: introduce sendfd() syscall (v.2)
Present patch introduces exceptionally easy to use, low latency and low overhead mechanism for transferring file descriptors between cooperating processes: int sendfd(pid_t pid, int sig, int fd) Given a target process pid, the sendfd() will queue a real-time signal for delivery to task referenced by pid. If signal can be delivered to destination tasks and it chooses to collect the associated signal info, a new file descriptor will be created on its behalf, pointing to file originally referred by fd (the value of newly created file descriptor will be communicated as integer payload within the siginfo data). Signed-off-by: Alex Dubov oa...@yahoo.com --- arch/x86/syscalls/syscall_32.tbl | 2 + arch/x86/syscalls/syscall_64.tbl | 1 + include/asm-generic/siginfo.h | 1 + include/linux/syscalls.h | 1 + include/uapi/asm-generic/siginfo.h | 1 + init/Kconfig | 11 + kernel/signal.c| 89 ++ kernel/sys_ni.c| 3 ++ 8 files changed, 109 insertions(+) diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl index 9fe1b5d..e2782bd 100644 --- a/arch/x86/syscalls/syscall_32.tbl +++ b/arch/x86/syscalls/syscall_32.tbl @@ -364,3 +364,5 @@ 355i386getrandom sys_getrandom 356i386memfd_createsys_memfd_create 357i386bpf sys_bpf +358i386sendfd sys_sendfd + diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl index 281150b..4d6b55d 100644 --- a/arch/x86/syscalls/syscall_64.tbl +++ b/arch/x86/syscalls/syscall_64.tbl @@ -328,6 +328,7 @@ 319common memfd_createsys_memfd_create 320common kexec_file_load sys_kexec_file_load 321common bpf sys_bpf +322common sendfd sys_sendfd # # x32-specific system call numbers start at 512 to avoid cache impact diff --git a/include/asm-generic/siginfo.h b/include/asm-generic/siginfo.h index 3d1a3af..c8af06f 100644 --- a/include/asm-generic/siginfo.h +++ b/include/asm-generic/siginfo.h @@ -12,6 +12,7 @@ #define __SI_RT(5 16) #define __SI_MESGQ (6 16) #define __SI_SYS (7 16) +#define __SI_FILEP (8 16) #define __SI_CODE(T,N) ((T) | ((N) 0x)) struct siginfo; diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index bda9b81..1871b72f 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -877,4 +877,5 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags, asmlinkage long sys_getrandom(char __user *buf, size_t count, unsigned int flags); asmlinkage long sys_bpf(int cmd, union bpf_attr *attr, unsigned int size); +asmlinkage long sys_sendfd(pid_t pid, int sig, int fd); #endif diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index ba5be7f..a92e38e 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -148,6 +148,7 @@ typedef struct siginfo { #define __SI_RT0 #define __SI_MESGQ 0 #define __SI_SYS 0 +#define __SI_FILEP 0 #define __SI_CODE(T,N) (N) #endif diff --git a/init/Kconfig b/init/Kconfig index 2081a4d..6a62a44 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1505,6 +1505,17 @@ config SIGNALFD If unsure, say Y. +config SENDFD + bool Enable sendfd() system call if EXPERT + default y + help + Enable the sendfd() system call that allows rapid duplication + of file descriptor across process boundaries. The target process + will receive a duplicate file descriptor delivered with one of + Posix.1b real-time signals. + + If unsure, say Y. + config TIMERFD bool Enable timerfd() system call if EXPERT select ANON_INODES diff --git a/kernel/signal.c b/kernel/signal.c index 8f0876f..299ee9c 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -35,6 +35,11 @@ #include linux/cn_proc.h #include linux/compiler.h +#ifdef CONFIG_SENDFD +#include linux/file.h +#include linux/fdtable.h +#endif + #define CREATE_TRACE_POINTS #include trace/events/signal.h @@ -394,8 +399,15 @@ __sigqueue_alloc(int sig, struct task_struct *t, gfp_t flags, int override_rlimi static void __sigqueue_free(struct sigqueue *q) { + if (q-info.si_code == __SI_FILEP) { + fput((struct file *)q-info.si_ptr); + q-info.si_code = 0; + q-info.si_ptr = NULL; + } + if (q-flags SIGQUEUE_PREALLOC) return; + atomic_dec(q-user-sigpending); free_uid(q-user); kmem_cache_free(sigqueue_cachep, q); @@ -543,6 +555,44 @@ unblock_all_signals(void) spin_unlock_irqrestore(current-sighand-siglock, flags); } +#ifdef CONFIG_SENDFD + +/* + * sendfd_copy_install can only
syscall: introduce sendfd() syscall (v.2)
I would like to present my second attempt at file descriptor duplication over Posix.1b real-time signal transport. All the constructive points raised in the previous discussion are believed to be addressed. To this end, I would like to address some concerns raised in the preceding discussion: 1. Claim: signals as a transport would not scale Each task_struct allocated by kernel has its own signal queue, reliable, when Posix.1b signals are concerned. This queue essentially serves as per-task mail box, enabling complex applications to send signals from each thread to each thread directly, with very low overhead, and thus avoid any shared contention points outright (originating task's pid is passed along with the siginfo data, so source based dispatching is perfectly possible). Also, signals can be trivially integrated with other communication mediums, as signalfd() syscall is perfectly compatible with epoll. 2. Claim: adding new functionality to the signal transport will create new attack/DoS vectors. Nothing can be further from truth. 2.a. If task A has sufficient capabilities to send signals to task B, then task A is already in position to do anything it wants with task B, including killing it outright. 2.b. Flood attacks on signal queues are not dangerous to the system, as signal queues are relatively shallow and consume little memory even when full. Compare with infamous recursive fd attack against AF_UNIX fd transport , which plagues application development to this day (due to safeguards introduced to alleviate it). 2.c. Natural decoupling of signal transport from vfs internals prevents any sort of recursive fd attacks altogether (it is even safe to send the signalfd() fd through - this can be considered a convenient feature to replicate signal delivery masks around; of course, the receiving task will only receive its own signals through it, peeking on other task's signals will not be possible). 3. Suggestion: new file desriptors should not appear in destination processes out of the blue. 3.a. To receive the signal, process must make non-trivial preparations ( manipulate signal masks, etc), which would only happen if certain signals are expected. 3.b. In present implementation, file desriptor is only created at the destination when destination task explictly elects to receive the associated signal info with sigtimedwait/signalfd. In the absence of destination task cooperation, the only overhead on the kernel side will be a single pair of ref_count increment/decrement, that is, completely negligible. 3.c. Due to the nature of siginfo delivery, operations on file descriptor table are completely safe and indistinguishable from a normal dup() system call. I would appreciate any additional constructive criticism, as it is in my interest as well to end up with safe and simple solution. However, I would prefer the criticism to target particular technical shortcomings, and not be derived from personal preferences, if possible. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 9:41 PM, Richard Cochran richardcoch...@gmail.com wrote: In any case, I find it hard to believe that the traditional method is really so bad. The explanation of why this new way is needed boils down to: unix programming is so hard to get right. Surely, this can be said about any new feature proposed. Why do we need this new thing called wheel? We lived 50k years without it just fine! It all boils down to: walking with legs is so hard to get right. :-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 2:40 PM, Al Viro wrote: > On Wed, Dec 03, 2014 at 01:22:33PM +1100, Alex Dubov wrote: > >> On a less related note, I hope you will agree that the simpler >> mechanism for this very in-demand feature is long overdue on Linux >> (every man and his dog are passing fds around these days). > > ... and I'm less than sure that it's a good thing. If nothing else, > once the pieces of your program are passing descriptors around freely, > you have created a barfball that will be impossible to split between > several boxen if you run into scalability issues. Descriptor-passing > is limited to a single system; you *can't* do that between e.g. components > of a cluster. So it's not an unmixed blessing, just as overuse of > shared memory segments, etc. They do have their uses, but that needs > to be carefully considered every time, or you'll create a major headache > a few years down the road. Well, if you try hard enough, you can pass fds around the components of the cluster - Mosix was doing just that some 10 years ago. Conceptually, it's even easier than doing distributed shared memory, as long as mmap is not concerned. :-) I was, however, looking at it from a different standpoint. Abundance of cores in the contemporary CPUs calls for locally parallel applications (and those are still the majority - clearly 90% of the applications and their workloads fit just fine on a single node). Thus, any modern application developer faces the usual dilemma: 1. Go multi-threaded - easy inter-thread IPC, lousy reliability with minor errors in secondary tasks crashing the whole application. 2. Go multi-process - circus hoop jumping when IPC is concerned, great reliability through OS provided fault isolation (so even really broken stuff, like PHP plugin for apache manages to perform most of the time :-) Memfd (on its own) and eventfd are great steps in the right direction, as managing persistent shmem and sem objects was always pain in the arse. If there was an alternative to AF_UNIX fd passing, with its arcane API, fs persistence and mind boggling fd recursion bugs, then option 2 would became much more attractive for developers leading to over-all increase in application reliability and security. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 4:00 AM, Al Viro wrote: > On Tue, Dec 02, 2014 at 03:35:18PM +1100, Alex Dubov wrote: >> + >> + if (rc < 0) >> + __close_fd(dst_files, s_info.si_int); > > Oh, lovely... And we are guaranteed that it still the same file, because...? > > Not to mention anything else, this stuff violates the assumption used in a lot > of places - that the *only* way for a process to modify a descriptor table is > to have a reference to it obtained by something that had it as its current > descriptor table and not dropped since then. The way you do it might actually > turn out to be OK, but there's no way I'll take that without detailed > analysis; > start with refcounting of struct file, for one thing - it does rely on the > assumption above in non-trivial ways. Ok, I see the problem here. This indeed requires further thought. > And that's aside of the points other folks had brought up. Yours is the first insightful message in this thread. Some of the other commenters exhibited an unfortunate lack of understanding, regarding what signals are and what they can be useful for. Unless, of course, I have missed something important. On a less related note, I hope you will agree that the simpler mechanism for this very in-demand feature is long overdue on Linux (every man and his dog are passing fds around these days). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 3:42 AM, Eric Dumazet wrote: > On Wed, 2014-12-03 at 03:23 +1100, Alex Dubov wrote: > > Tell me how a 128 threads program can use this new mechanism in a > scalable way. > > One signal per thread ? What for? Kernel will deliver the signal only to the thread/threads which has the relevant signal unblocked (they are blocked by default). > > I guess we'll keep AF_UNIX then, thank you. Kindly enlighten me, how are you going to use any file descriptor in a 128 threads program in a scalable way (socket and all)? How this approach will be different when using signalfd()? And no, I'm not proposing to take your favorite toys away. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Minimal effort/low overhead file descriptor duplication over Posix.1b s
On Wed, Dec 3, 2014 at 2:26 AM, Jonathan Corbet wrote: > On Tue, 2 Dec 2014 15:35:17 +1100 > Alex Dubov wrote: > > > - Messing with another process's file descriptor table without its >knowledge looks like a possible source of all kinds problems. Might >there be race conditions with close()/dup() code, for example? And >remember that users can be root in a user namespace; maybe there's no >potential for mischief there, but it needs to be considered. If process A has sufficient permissions to signal process B, it can already do arbitrary mischief, no news there (SIGKILL and SIGSTOP will definitely cause more havoc :-). I don't believe there can be any race conditions as this is not different to what happens when dup() is invoked from one of the threads in multi-threaded application, whereupon other threads go on with their usual file operations. Descriptor duplication happens prior to any signal handling activities. > - Forcing the use of realtime signals seems strange; this isn't a >realtime operation by any stretch. "Real time signals" are merely a misleading name for Posix.1b micro-messaging facility. To the best of my knowledge they do not affect scheduling any more then SIGIO or SIGALRM would. As Posix.1b signals are best handled by signalfd() facility anyway, no impact on scheduling compared to any other approach (including the existing domain socket approach) is expected at all. > > - How might the sending process communicate to the recipient what the fd >is for? Even if a process only expects one type of file descriptor, >the ability to communicate information other than its number seems >like it would often be useful. There are 32 "real time" signals defined by default in kernel; this range can be increased at will with kernel recompilation and glibc will pick up the correct range automatically (this is Posix mandated behavior and it actually works like that). I have not seen an app yet that relied on more than half a dozen of distinct signal numbers. Thus any application can conveniently define more than 2 dozens of different fd varieties out of the box, delivered to it with dedicated signal ids, whereupon in most practical applications only 1 or 2 varieties of file descriptors are ever passed around. > > Some of these concerns might be addressable by requiring the recipient to > call acceptfd() (or some such) with the ability to use poll(). As an > alternative, I believe kdbus has fd-passing abilities; if kdbus goes in, > would you still need this feature? Any process willing to handle Posix.1b signals must explicitly manipulate the signal masks - otherwise it will be killed the moment signal is received. Thus, no special "acceptfd()" call is necessary on the receiver side - applications usually don't modify their signal masks unless they expect some particular signal to arrive. kdbus has something like it and binder on android has it as well. The problem with both of them are the same as with unix domain sockets (which implement a whole, rather convoluted, cmsg facility to be ever used for that single purpose): they try to solve big problems with fancy functionality, whereupon fd passing is a nice side feature (which then gets used the most). To my understanding, commonly used functionality deserves to have its own quick, low overhead path: 1. We've got eventfd() which is neat and all, but to use it we need an easy way to pass its fd around. 2. We've got memfd() which is also neat, but to use it.. 3. We've got fairly complex (and consequently buggy) functionality like SO_REUSEPORT, but I can't avoid a feeling that if there was a low overhead transport available to path fds around (like the one proposed), the old school approach of having one process running tightly around accept() and sending sockets to workers may still rival it (pity I don't have google's setup around to test it). 4. Most importantly, when network appliances are concerned (and those represent a huge percentage of linux install base), it is desirable to have the leanest possible code paths both in kernel and in the user space (no functionality - no vulnerabilities to fish for) and still be able to rely on multi-process applications (as multi-process applications are considerably more reliable then multi-threaded ones, for all the obvious reasons). A compact, easily traceable facility comprising few hundred LOCs in the kernel, end to end, and very simple application code (sigqueue() -> signalfd()) pose a distinct advantage in this regard over largish subsystems which may provide similar feature (invariable at the expense of unnecessary costs, like persistent file system objects, specialized user-space libraries, etc) . -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 2:33 AM, Eric Dumazet wrote: > On Wed, 2014-12-03 at 01:47 +1100, Alex Dubov wrote: >> > User A can send fd(s) to processes belonging to user B, even if user B >> > does (probably) not want this to happen ? >> >> 1. Process A must have sufficient permissions to signal process B. >> This will only happen if process A belongs to the same user as process >> B or has elevated capabilities, which can not appear by themselves >> (and if root on some machine can not be trusted, then all is lost >> anyway). >> > > I do not see this enforced in your patch. > > Allowing a process to hold many times the lock protecting my file > descriptor table is very scary. > > Reserving a slot, then undo this if the signal failed is a nice way to > slow down critical programs and eventually block them from doing > progress when using file descriptors (most system calls afaik) Yes, this is an omission. I already promised to tighten the security in my last post. :) >> 2. If process B has not specified explicitly how it wants the >> particular signal to be handled, it will be killed by the default >> handler. End of story, nothing else is going to happen. > > So it seems possible for an arbitrary program to send fds to innocent > programs, that will likely fill their fd table and wont be able to open > a new file. > > This opens interesting security issues and attack vectors. Same as SIGKILL. And yet, our machines are still working fine. If process A has sufficient capability to send signals to process B, then process B is already at its mercy, fds or not fds. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
> User A can send fd(s) to processes belonging to user B, even if user B > does (probably) not want this to happen ? 1. Process A must have sufficient permissions to signal process B. This will only happen if process A belongs to the same user as process B or has elevated capabilities, which can not appear by themselves (and if root on some machine can not be trusted, then all is lost anyway). 2. If process B has not specified explicitly how it wants the particular signal to be handled, it will be killed by the default handler. End of story, nothing else is going to happen. I suppose, I can add an extra permissions check prior to creating the new file descriptor in the first place. > Also, relying on signals seems quite old fashion these days. How about > multi-threaded programs wanting separate channels to receive fds ? Most multi-threaded programs share the same file table between all threads (unless some fancy clone() magic is involved), so the issue is rather mundane. At any rate, each thread has its own pid and the usual signal routing applies. At a more generic level Posix real-time signals are anything, but old-fashioned. sigqueue()/signalfd() pair provides a very convenient, low overhead micro-messaging facility with ordered, reliably delivery. I fail to see what's wrong with making a worthy use of it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] fs: Wire up sendfd() syscall (all architectures)
On Tue, Dec 2, 2014 at 10:42 PM, Michal Simek wrote: > On 12/02/2014 09:01 AM, Geert Uytterhoeven wrote: >> This really needs a CC to linux-arch (added). >> >> On Tue, Dec 2, 2014 at 5:35 AM, Alex Dubov wrote: >>> Signed-off-by: Alex Dubov >>> --- > > The same for microblaze here > arch/microblaze/include/asm/unistd.h > This invites the question as to why the __NR_syscalls macro is not defined in uapi/asm/unistd.h on those architectures, where it will be easier to spot? After all, asm/unistd.h includes uapi/asm/unistd.h unconditionally. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] fs: Wire up sendfd() syscall (all architectures)
On Tue, Dec 2, 2014 at 7:01 PM, Geert Uytterhoeven wrote: > This really needs a CC to linux-arch (added). > > > You forgot to update NR_syscalls in arch/m68k/include/asm/unistd.h. > Noted. I would assume that other architectures may have similar problems (I only tested my submission on x86_64). Will try to fix those when/if there's progress toward accepting the proposed feature. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] fs: Wire up sendfd() syscall (all architectures)
On Tue, Dec 2, 2014 at 7:01 PM, Geert Uytterhoeven ge...@linux-m68k.org wrote: This really needs a CC to linux-arch (added). You forgot to update NR_syscalls in arch/m68k/include/asm/unistd.h. Noted. I would assume that other architectures may have similar problems (I only tested my submission on x86_64). Will try to fix those when/if there's progress toward accepting the proposed feature. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] fs: Wire up sendfd() syscall (all architectures)
On Tue, Dec 2, 2014 at 10:42 PM, Michal Simek mon...@monstr.eu wrote: On 12/02/2014 09:01 AM, Geert Uytterhoeven wrote: This really needs a CC to linux-arch (added). On Tue, Dec 2, 2014 at 5:35 AM, Alex Dubov alex.du...@gmail.com wrote: Signed-off-by: Alex Dubov oa...@yahoo.com --- The same for microblaze here arch/microblaze/include/asm/unistd.h This invites the question as to why the __NR_syscalls macro is not defined in uapi/asm/unistd.h on those architectures, where it will be easier to spot? After all, asm/unistd.h includes uapi/asm/unistd.h unconditionally. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
User A can send fd(s) to processes belonging to user B, even if user B does (probably) not want this to happen ? 1. Process A must have sufficient permissions to signal process B. This will only happen if process A belongs to the same user as process B or has elevated capabilities, which can not appear by themselves (and if root on some machine can not be trusted, then all is lost anyway). 2. If process B has not specified explicitly how it wants the particular signal to be handled, it will be killed by the default handler. End of story, nothing else is going to happen. I suppose, I can add an extra permissions check prior to creating the new file descriptor in the first place. Also, relying on signals seems quite old fashion these days. How about multi-threaded programs wanting separate channels to receive fds ? Most multi-threaded programs share the same file table between all threads (unless some fancy clone() magic is involved), so the issue is rather mundane. At any rate, each thread has its own pid and the usual signal routing applies. At a more generic level Posix real-time signals are anything, but old-fashioned. sigqueue()/signalfd() pair provides a very convenient, low overhead micro-messaging facility with ordered, reliably delivery. I fail to see what's wrong with making a worthy use of it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 2:33 AM, Eric Dumazet eric.duma...@gmail.com wrote: On Wed, 2014-12-03 at 01:47 +1100, Alex Dubov wrote: User A can send fd(s) to processes belonging to user B, even if user B does (probably) not want this to happen ? 1. Process A must have sufficient permissions to signal process B. This will only happen if process A belongs to the same user as process B or has elevated capabilities, which can not appear by themselves (and if root on some machine can not be trusted, then all is lost anyway). I do not see this enforced in your patch. Allowing a process to hold many times the lock protecting my file descriptor table is very scary. Reserving a slot, then undo this if the signal failed is a nice way to slow down critical programs and eventually block them from doing progress when using file descriptors (most system calls afaik) Yes, this is an omission. I already promised to tighten the security in my last post. :) 2. If process B has not specified explicitly how it wants the particular signal to be handled, it will be killed by the default handler. End of story, nothing else is going to happen. So it seems possible for an arbitrary program to send fds to innocent programs, that will likely fill their fd table and wont be able to open a new file. This opens interesting security issues and attack vectors. Same as SIGKILL. And yet, our machines are still working fine. If process A has sufficient capability to send signals to process B, then process B is already at its mercy, fds or not fds. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Minimal effort/low overhead file descriptor duplication over Posix.1b s
On Wed, Dec 3, 2014 at 2:26 AM, Jonathan Corbet cor...@lwn.net wrote: On Tue, 2 Dec 2014 15:35:17 +1100 Alex Dubov alex.du...@gmail.com wrote: - Messing with another process's file descriptor table without its knowledge looks like a possible source of all kinds problems. Might there be race conditions with close()/dup() code, for example? And remember that users can be root in a user namespace; maybe there's no potential for mischief there, but it needs to be considered. If process A has sufficient permissions to signal process B, it can already do arbitrary mischief, no news there (SIGKILL and SIGSTOP will definitely cause more havoc :-). I don't believe there can be any race conditions as this is not different to what happens when dup() is invoked from one of the threads in multi-threaded application, whereupon other threads go on with their usual file operations. Descriptor duplication happens prior to any signal handling activities. - Forcing the use of realtime signals seems strange; this isn't a realtime operation by any stretch. Real time signals are merely a misleading name for Posix.1b micro-messaging facility. To the best of my knowledge they do not affect scheduling any more then SIGIO or SIGALRM would. As Posix.1b signals are best handled by signalfd() facility anyway, no impact on scheduling compared to any other approach (including the existing domain socket approach) is expected at all. - How might the sending process communicate to the recipient what the fd is for? Even if a process only expects one type of file descriptor, the ability to communicate information other than its number seems like it would often be useful. There are 32 real time signals defined by default in kernel; this range can be increased at will with kernel recompilation and glibc will pick up the correct range automatically (this is Posix mandated behavior and it actually works like that). I have not seen an app yet that relied on more than half a dozen of distinct signal numbers. Thus any application can conveniently define more than 2 dozens of different fd varieties out of the box, delivered to it with dedicated signal ids, whereupon in most practical applications only 1 or 2 varieties of file descriptors are ever passed around. Some of these concerns might be addressable by requiring the recipient to call acceptfd() (or some such) with the ability to use poll(). As an alternative, I believe kdbus has fd-passing abilities; if kdbus goes in, would you still need this feature? Any process willing to handle Posix.1b signals must explicitly manipulate the signal masks - otherwise it will be killed the moment signal is received. Thus, no special acceptfd() call is necessary on the receiver side - applications usually don't modify their signal masks unless they expect some particular signal to arrive. kdbus has something like it and binder on android has it as well. The problem with both of them are the same as with unix domain sockets (which implement a whole, rather convoluted, cmsg facility to be ever used for that single purpose): they try to solve big problems with fancy functionality, whereupon fd passing is a nice side feature (which then gets used the most). To my understanding, commonly used functionality deserves to have its own quick, low overhead path: 1. We've got eventfd() which is neat and all, but to use it we need an easy way to pass its fd around. 2. We've got memfd() which is also neat, but to use it.. 3. We've got fairly complex (and consequently buggy) functionality like SO_REUSEPORT, but I can't avoid a feeling that if there was a low overhead transport available to path fds around (like the one proposed), the old school approach of having one process running tightly around accept() and sending sockets to workers may still rival it (pity I don't have google's setup around to test it). 4. Most importantly, when network appliances are concerned (and those represent a huge percentage of linux install base), it is desirable to have the leanest possible code paths both in kernel and in the user space (no functionality - no vulnerabilities to fish for) and still be able to rely on multi-process applications (as multi-process applications are considerably more reliable then multi-threaded ones, for all the obvious reasons). A compact, easily traceable facility comprising few hundred LOCs in the kernel, end to end, and very simple application code (sigqueue() - signalfd()) pose a distinct advantage in this regard over largish subsystems which may provide similar feature (invariable at the expense of unnecessary costs, like persistent file system objects, specialized user-space libraries, etc) . -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 3:42 AM, Eric Dumazet eric.duma...@gmail.com wrote: On Wed, 2014-12-03 at 03:23 +1100, Alex Dubov wrote: Tell me how a 128 threads program can use this new mechanism in a scalable way. One signal per thread ? What for? Kernel will deliver the signal only to the thread/threads which has the relevant signal unblocked (they are blocked by default). I guess we'll keep AF_UNIX then, thank you. Kindly enlighten me, how are you going to use any file descriptor in a 128 threads program in a scalable way (socket and all)? How this approach will be different when using signalfd()? And no, I'm not proposing to take your favorite toys away. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 4:00 AM, Al Viro v...@zeniv.linux.org.uk wrote: On Tue, Dec 02, 2014 at 03:35:18PM +1100, Alex Dubov wrote: + + if (rc 0) + __close_fd(dst_files, s_info.si_int); Oh, lovely... And we are guaranteed that it still the same file, because...? Not to mention anything else, this stuff violates the assumption used in a lot of places - that the *only* way for a process to modify a descriptor table is to have a reference to it obtained by something that had it as its current descriptor table and not dropped since then. The way you do it might actually turn out to be OK, but there's no way I'll take that without detailed analysis; start with refcounting of struct file, for one thing - it does rely on the assumption above in non-trivial ways. Ok, I see the problem here. This indeed requires further thought. And that's aside of the points other folks had brought up. Yours is the first insightful message in this thread. Some of the other commenters exhibited an unfortunate lack of understanding, regarding what signals are and what they can be useful for. Unless, of course, I have missed something important. On a less related note, I hope you will agree that the simpler mechanism for this very in-demand feature is long overdue on Linux (every man and his dog are passing fds around these days). -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fs: introduce sendfd() syscall
On Wed, Dec 3, 2014 at 2:40 PM, Al Viro v...@zeniv.linux.org.uk wrote: On Wed, Dec 03, 2014 at 01:22:33PM +1100, Alex Dubov wrote: On a less related note, I hope you will agree that the simpler mechanism for this very in-demand feature is long overdue on Linux (every man and his dog are passing fds around these days). ... and I'm less than sure that it's a good thing. If nothing else, once the pieces of your program are passing descriptors around freely, you have created a barfball that will be impossible to split between several boxen if you run into scalability issues. Descriptor-passing is limited to a single system; you *can't* do that between e.g. components of a cluster. So it's not an unmixed blessing, just as overuse of shared memory segments, etc. They do have their uses, but that needs to be carefully considered every time, or you'll create a major headache a few years down the road. Well, if you try hard enough, you can pass fds around the components of the cluster - Mosix was doing just that some 10 years ago. Conceptually, it's even easier than doing distributed shared memory, as long as mmap is not concerned. :-) I was, however, looking at it from a different standpoint. Abundance of cores in the contemporary CPUs calls for locally parallel applications (and those are still the majority - clearly 90% of the applications and their workloads fit just fine on a single node). Thus, any modern application developer faces the usual dilemma: 1. Go multi-threaded - easy inter-thread IPC, lousy reliability with minor errors in secondary tasks crashing the whole application. 2. Go multi-process - circus hoop jumping when IPC is concerned, great reliability through OS provided fault isolation (so even really broken stuff, like PHP plugin for apache manages to perform most of the time :-) Memfd (on its own) and eventfd are great steps in the right direction, as managing persistent shmem and sem objects was always pain in the arse. If there was an alternative to AF_UNIX fd passing, with its arcane API, fs persistence and mind boggling fd recursion bugs, then option 2 would became much more attractive for developers leading to over-all increase in application reliability and security. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] fs: introduce sendfd() syscall
Present patch introduces exceptionally easy to use, low latency and low overhead mechanism for transferring file descriptors between cooperating processes: int sendfd(pid_t pid, int sig, int fd) Given a target process pid, the sendfd() syscall will create a duplicate file descriptor in a target task's (referred by pid) file table pointing to the file references by descriptor fd. Then, it will attempt to notify the target task by issuing a Posix.1b real-time signal (sig), carrying the new file descriptor as integer payload. If real-time signal can not be enqueued at the destination signal queue, the newly created file descriptor will be promptly closed. Signed-off-by: Alex Dubov --- fs/Makefile | 1 + fs/sendfd.c | 82 init/Kconfig | 11 3 files changed, 94 insertions(+) create mode 100644 fs/sendfd.c diff --git a/fs/Makefile b/fs/Makefile index da0bbb4..bed05a8 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_ANON_INODES) += anon_inodes.o obj-$(CONFIG_SIGNALFD) += signalfd.o obj-$(CONFIG_TIMERFD) += timerfd.o obj-$(CONFIG_EVENTFD) += eventfd.o +obj-$(CONFIG_SENDFD) += sendfd.o obj-$(CONFIG_AIO) += aio.o obj-$(CONFIG_FILE_LOCKING) += locks.o obj-$(CONFIG_COMPAT) += compat.o compat_ioctl.o diff --git a/fs/sendfd.c b/fs/sendfd.c new file mode 100644 index 000..1e85484 --- /dev/null +++ b/fs/sendfd.c @@ -0,0 +1,82 @@ +/* + * fs/sendfd.c + * + * Copyright (C) 2014 Alex Dubov + * + */ + +#include +#include +#include + +SYSCALL_DEFINE3(sendfd, pid_t, pid, int, sig, int, fd) +{ + struct siginfo s_info = { + .si_signo = sig, + .si_errno = 0, + .si_code = __SI_RT + }; + struct file *src_file = NULL; + struct task_struct *dst_task = NULL; + struct files_struct *dst_files = NULL; + unsigned long rlim = 0; + unsigned long flags = 0; + int rc = 0; + + if ((sig < SIGRTMIN) || (sig > SIGRTMAX)) + return -EINVAL; + + s_info.si_pid = task_pid_vnr(current); + s_info.si_uid = from_kuid_munged(current_user_ns(), current_uid()); + s_info.si_int = -1; + + src_file = fget(fd); + if (!src_file) + return -EBADF; + + rcu_read_lock(); + dst_task = find_task_by_vpid(pid); + + if (!dst_task) { + rc = -ESRCH; + goto out_put_src_file; + } + get_task_struct(dst_task); + rcu_read_unlock(); + + dst_files = get_files_struct(dst_task); + if (!dst_files) { + rc = -EMFILE; + goto out_put_dst_task; + } + + if (!lock_task_sighand(dst_task, )) { + rc = -EMFILE; + goto out_put_dst_files; + } + + rlim = task_rlimit(dst_task, RLIMIT_NOFILE); + + unlock_task_sighand(dst_task, ); + + rc = __alloc_fd(dst_task->files, 0, rlim, O_CLOEXEC); + if (rc < 0) + goto out_put_dst_files; + + s_info.si_int = rc; + + get_file(src_file); + __fd_install(dst_files, rc, src_file); + rc = kill_pid_info(sig, _info, task_pid(dst_task)); + + if (rc < 0) + __close_fd(dst_files, s_info.si_int); + +out_put_dst_files: + put_files_struct(dst_files); +out_put_dst_task: + put_task_struct(dst_task); +out_put_src_file: + fput(src_file); + return rc; +} diff --git a/init/Kconfig b/init/Kconfig index 2081a4d..dfe8b6f 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1525,6 +1525,17 @@ config EVENTFD If unsure, say Y. +config SENDFD + bool "Enable sendfd() system call" if EXPERT + default y + help + Enable the sendfd() system call that allows rapid duplication + of file descriptor across process boundaries. The target process + will receive a duplicate file descriptor delivered with one of + Posix.1b real-time signals. + + If unsure, say Y. + # syscall, maps, verifier config BPF_SYSCALL bool "Enable bpf() system call" if EXPERT -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] fs: Wire up sendfd() syscall (all architectures)
Signed-off-by: Alex Dubov --- arch/arm/include/uapi/asm/unistd.h| 1 + arch/arm/kernel/calls.S | 1 + arch/arm64/include/asm/unistd32.h | 2 ++ arch/ia64/include/uapi/asm/unistd.h | 1 + arch/ia64/kernel/entry.S | 1 + arch/m68k/include/uapi/asm/unistd.h | 1 + arch/m68k/kernel/syscalltable.S | 1 + arch/microblaze/include/uapi/asm/unistd.h | 1 + arch/microblaze/kernel/syscall_table.S| 1 + arch/mips/include/uapi/asm/unistd.h | 15 +-- arch/mips/kernel/scall32-o32.S| 1 + arch/mips/kernel/scall64-64.S | 1 + arch/mips/kernel/scall64-n32.S| 1 + arch/mips/kernel/scall64-o32.S| 1 + arch/parisc/include/uapi/asm/unistd.h | 3 ++- arch/powerpc/include/asm/systbl.h | 1 + arch/powerpc/include/uapi/asm/unistd.h| 1 + arch/s390/include/uapi/asm/unistd.h | 3 ++- arch/s390/kernel/compat_wrapper.c | 1 + arch/s390/kernel/syscalls.S | 1 + arch/sparc/include/uapi/asm/unistd.h | 3 ++- arch/sparc/kernel/systbls_32.S| 2 +- arch/sparc/kernel/systbls_64.S| 4 ++-- arch/x86/syscalls/syscall_32.tbl | 1 + arch/x86/syscalls/syscall_64.tbl | 1 + arch/xtensa/include/uapi/asm/unistd.h | 5 +++-- include/linux/syscalls.h | 1 + include/uapi/asm-generic/unistd.h | 4 +++- kernel/sys_ni.c | 3 +++ 29 files changed, 48 insertions(+), 15 deletions(-) diff --git a/arch/arm/include/uapi/asm/unistd.h b/arch/arm/include/uapi/asm/unistd.h index 705bb76..6428823 100644 --- a/arch/arm/include/uapi/asm/unistd.h +++ b/arch/arm/include/uapi/asm/unistd.h @@ -413,6 +413,7 @@ #define __NR_getrandom (__NR_SYSCALL_BASE+384) #define __NR_memfd_create (__NR_SYSCALL_BASE+385) #define __NR_bpf (__NR_SYSCALL_BASE+386) +#define __NR_sendfd(__NR_SYSCALL_BASE+387) /* * The following SWIs are ARM private. diff --git a/arch/arm/kernel/calls.S b/arch/arm/kernel/calls.S index e51833f..30bdeb5 100644 --- a/arch/arm/kernel/calls.S +++ b/arch/arm/kernel/calls.S @@ -396,6 +396,7 @@ CALL(sys_getrandom) /* 385 */ CALL(sys_memfd_create) CALL(sys_bpf) + CALL(sys_sendfd) #ifndef syscalls_counted .equ syscalls_padding, ((NR_syscalls + 3) & ~3) - NR_syscalls #define syscalls_counted diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h index 9dfdac4..7f19595 100644 --- a/arch/arm64/include/asm/unistd32.h +++ b/arch/arm64/include/asm/unistd32.h @@ -794,3 +794,5 @@ __SYSCALL(__NR_getrandom, sys_getrandom) __SYSCALL(__NR_memfd_create, sys_memfd_create) #define __NR_bpf 386 __SYSCALL(__NR_bpf, sys_bpf) +#define __NR_sendfd 387 +__SYSCALL(__NR_sendfd, sys_sendfd) diff --git a/arch/ia64/include/uapi/asm/unistd.h b/arch/ia64/include/uapi/asm/unistd.h index 4c2240c..55be68c 100644 --- a/arch/ia64/include/uapi/asm/unistd.h +++ b/arch/ia64/include/uapi/asm/unistd.h @@ -331,5 +331,6 @@ #define __NR_getrandom 1339 #define __NR_memfd_create 1340 #define __NR_bpf 1341 +#define __NR_sendfd1342 #endif /* _UAPI_ASM_IA64_UNISTD_H */ diff --git a/arch/ia64/kernel/entry.S b/arch/ia64/kernel/entry.S index f5e96df..97596a3 100644 --- a/arch/ia64/kernel/entry.S +++ b/arch/ia64/kernel/entry.S @@ -1779,6 +1779,7 @@ sys_call_table: data8 sys_getrandom data8 sys_memfd_create // 1340 data8 sys_bpf + data8 sys_sendfd .org sys_call_table + 8*NR_syscalls // guard against failures to increase NR_syscalls #endif /* __IA64_ASM_PARAVIRTUALIZED_NATIVE */ diff --git a/arch/m68k/include/uapi/asm/unistd.h b/arch/m68k/include/uapi/asm/unistd.h index 2c1bec9..77e7098 100644 --- a/arch/m68k/include/uapi/asm/unistd.h +++ b/arch/m68k/include/uapi/asm/unistd.h @@ -360,5 +360,6 @@ #define __NR_getrandom 352 #define __NR_memfd_create 353 #define __NR_bpf 354 +#define __NR_sendfd355 #endif /* _UAPI_ASM_M68K_UNISTD_H_ */ diff --git a/arch/m68k/kernel/syscalltable.S b/arch/m68k/kernel/syscalltable.S index 2ca219e..3ea20d4 100644 --- a/arch/m68k/kernel/syscalltable.S +++ b/arch/m68k/kernel/syscalltable.S @@ -375,4 +375,5 @@ ENTRY(sys_call_table) .long sys_getrandom .long sys_memfd_create .long sys_bpf + .long sys_sendfd diff --git a/arch/microblaze/include/uapi/asm/unistd.h b/arch/microblaze/include/uapi/asm/unistd.h index c712677..f69e30a 100644 --- a/arch/microblaze/include/uapi/asm/unistd.h +++ b/arch/microblaze/include/uapi/asm/unistd.h @@ -403,5 +403,6 @@ #define __NR_getrandom 385 #define __NR_memfd_create 386 #define __NR_bpf 387 +#define __NR_sendfd388 #e
Minimal effort/low overhead file descriptor duplication over Posix.1b s
A common requirement in parallel processing applications (relied upon by popular network servers, databases and various other applications) is to pass open file descriptors between processes. Historically, several mechanisms existed to support this requirement, such as those provided by "cmsg" facility of unix domain sockets or special operations on named pipes (on Android this can also be achieved using "binder" facility). Unfortunately, using facilities like Unix domain sockets to merely pass file descriptors between "worker" processes is unnecessarily difficult, due to the following common consideration: 1. Domain sockets and named pipes are persistent objects. Applications must manage their lifetime and devise unambiguous access schemes in case multiple application instances are to be run within the same OS instance. Usually, they would also require a writable file system to be mounted. 2. Interaction with domain sockets and named pipes requires a sizable, non-trivial and error-prone code on the application side, especially in cases where multiple worker types started by multiple application instances must coexist within the same OS instance. 3. Domain sockets and pipes require creation of complex kernel-side set-ups, whereupon, in many cases, the only information ever passed by the application over those channels are file descriptors (it is usual for the major part of the application's shared state to be established through other mechanisms, like shared memory). In some cases, applications are forced to send meaningless rubbish over the domain socket merely to "push" the associated "cmsg" carrying the file descriptor through. Present patch introduces exceptionally easy to use, low latency and low overhead mechanism for transferring file descriptors between cooperating processes: int sendfd(pid_t pid, int sig, int fd) Given a target process pid, the sendfd() syscall will create a duplicate file descriptor in a target task's (referred by pid) file table pointing to the file references by descriptor fd. Then, it will attempt to notify the target task by issuing a Posix.1b real-time signal (sig), carrying the new file descriptor as integer payload. If real-time signal can not be enqueued at the destination signal queue, the newly created file descriptor will be promptly closed. It is believed, that proposed sendfd() syscall, together with recently accepted "memfd" facility may greatly simplify development of parallel processing applications, by eliminating the need to rely on tricky and possibly insecure approaches involving domain sockets and such. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] fs: Wire up sendfd() syscall (all architectures)
Signed-off-by: Alex Dubov oa...@yahoo.com --- arch/arm/include/uapi/asm/unistd.h| 1 + arch/arm/kernel/calls.S | 1 + arch/arm64/include/asm/unistd32.h | 2 ++ arch/ia64/include/uapi/asm/unistd.h | 1 + arch/ia64/kernel/entry.S | 1 + arch/m68k/include/uapi/asm/unistd.h | 1 + arch/m68k/kernel/syscalltable.S | 1 + arch/microblaze/include/uapi/asm/unistd.h | 1 + arch/microblaze/kernel/syscall_table.S| 1 + arch/mips/include/uapi/asm/unistd.h | 15 +-- arch/mips/kernel/scall32-o32.S| 1 + arch/mips/kernel/scall64-64.S | 1 + arch/mips/kernel/scall64-n32.S| 1 + arch/mips/kernel/scall64-o32.S| 1 + arch/parisc/include/uapi/asm/unistd.h | 3 ++- arch/powerpc/include/asm/systbl.h | 1 + arch/powerpc/include/uapi/asm/unistd.h| 1 + arch/s390/include/uapi/asm/unistd.h | 3 ++- arch/s390/kernel/compat_wrapper.c | 1 + arch/s390/kernel/syscalls.S | 1 + arch/sparc/include/uapi/asm/unistd.h | 3 ++- arch/sparc/kernel/systbls_32.S| 2 +- arch/sparc/kernel/systbls_64.S| 4 ++-- arch/x86/syscalls/syscall_32.tbl | 1 + arch/x86/syscalls/syscall_64.tbl | 1 + arch/xtensa/include/uapi/asm/unistd.h | 5 +++-- include/linux/syscalls.h | 1 + include/uapi/asm-generic/unistd.h | 4 +++- kernel/sys_ni.c | 3 +++ 29 files changed, 48 insertions(+), 15 deletions(-) diff --git a/arch/arm/include/uapi/asm/unistd.h b/arch/arm/include/uapi/asm/unistd.h index 705bb76..6428823 100644 --- a/arch/arm/include/uapi/asm/unistd.h +++ b/arch/arm/include/uapi/asm/unistd.h @@ -413,6 +413,7 @@ #define __NR_getrandom (__NR_SYSCALL_BASE+384) #define __NR_memfd_create (__NR_SYSCALL_BASE+385) #define __NR_bpf (__NR_SYSCALL_BASE+386) +#define __NR_sendfd(__NR_SYSCALL_BASE+387) /* * The following SWIs are ARM private. diff --git a/arch/arm/kernel/calls.S b/arch/arm/kernel/calls.S index e51833f..30bdeb5 100644 --- a/arch/arm/kernel/calls.S +++ b/arch/arm/kernel/calls.S @@ -396,6 +396,7 @@ CALL(sys_getrandom) /* 385 */ CALL(sys_memfd_create) CALL(sys_bpf) + CALL(sys_sendfd) #ifndef syscalls_counted .equ syscalls_padding, ((NR_syscalls + 3) ~3) - NR_syscalls #define syscalls_counted diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h index 9dfdac4..7f19595 100644 --- a/arch/arm64/include/asm/unistd32.h +++ b/arch/arm64/include/asm/unistd32.h @@ -794,3 +794,5 @@ __SYSCALL(__NR_getrandom, sys_getrandom) __SYSCALL(__NR_memfd_create, sys_memfd_create) #define __NR_bpf 386 __SYSCALL(__NR_bpf, sys_bpf) +#define __NR_sendfd 387 +__SYSCALL(__NR_sendfd, sys_sendfd) diff --git a/arch/ia64/include/uapi/asm/unistd.h b/arch/ia64/include/uapi/asm/unistd.h index 4c2240c..55be68c 100644 --- a/arch/ia64/include/uapi/asm/unistd.h +++ b/arch/ia64/include/uapi/asm/unistd.h @@ -331,5 +331,6 @@ #define __NR_getrandom 1339 #define __NR_memfd_create 1340 #define __NR_bpf 1341 +#define __NR_sendfd1342 #endif /* _UAPI_ASM_IA64_UNISTD_H */ diff --git a/arch/ia64/kernel/entry.S b/arch/ia64/kernel/entry.S index f5e96df..97596a3 100644 --- a/arch/ia64/kernel/entry.S +++ b/arch/ia64/kernel/entry.S @@ -1779,6 +1779,7 @@ sys_call_table: data8 sys_getrandom data8 sys_memfd_create // 1340 data8 sys_bpf + data8 sys_sendfd .org sys_call_table + 8*NR_syscalls // guard against failures to increase NR_syscalls #endif /* __IA64_ASM_PARAVIRTUALIZED_NATIVE */ diff --git a/arch/m68k/include/uapi/asm/unistd.h b/arch/m68k/include/uapi/asm/unistd.h index 2c1bec9..77e7098 100644 --- a/arch/m68k/include/uapi/asm/unistd.h +++ b/arch/m68k/include/uapi/asm/unistd.h @@ -360,5 +360,6 @@ #define __NR_getrandom 352 #define __NR_memfd_create 353 #define __NR_bpf 354 +#define __NR_sendfd355 #endif /* _UAPI_ASM_M68K_UNISTD_H_ */ diff --git a/arch/m68k/kernel/syscalltable.S b/arch/m68k/kernel/syscalltable.S index 2ca219e..3ea20d4 100644 --- a/arch/m68k/kernel/syscalltable.S +++ b/arch/m68k/kernel/syscalltable.S @@ -375,4 +375,5 @@ ENTRY(sys_call_table) .long sys_getrandom .long sys_memfd_create .long sys_bpf + .long sys_sendfd diff --git a/arch/microblaze/include/uapi/asm/unistd.h b/arch/microblaze/include/uapi/asm/unistd.h index c712677..f69e30a 100644 --- a/arch/microblaze/include/uapi/asm/unistd.h +++ b/arch/microblaze/include/uapi/asm/unistd.h @@ -403,5 +403,6 @@ #define __NR_getrandom 385 #define __NR_memfd_create 386 #define __NR_bpf 387 +#define __NR_sendfd388
Minimal effort/low overhead file descriptor duplication over Posix.1b s
A common requirement in parallel processing applications (relied upon by popular network servers, databases and various other applications) is to pass open file descriptors between processes. Historically, several mechanisms existed to support this requirement, such as those provided by cmsg facility of unix domain sockets or special operations on named pipes (on Android this can also be achieved using binder facility). Unfortunately, using facilities like Unix domain sockets to merely pass file descriptors between worker processes is unnecessarily difficult, due to the following common consideration: 1. Domain sockets and named pipes are persistent objects. Applications must manage their lifetime and devise unambiguous access schemes in case multiple application instances are to be run within the same OS instance. Usually, they would also require a writable file system to be mounted. 2. Interaction with domain sockets and named pipes requires a sizable, non-trivial and error-prone code on the application side, especially in cases where multiple worker types started by multiple application instances must coexist within the same OS instance. 3. Domain sockets and pipes require creation of complex kernel-side set-ups, whereupon, in many cases, the only information ever passed by the application over those channels are file descriptors (it is usual for the major part of the application's shared state to be established through other mechanisms, like shared memory). In some cases, applications are forced to send meaningless rubbish over the domain socket merely to push the associated cmsg carrying the file descriptor through. Present patch introduces exceptionally easy to use, low latency and low overhead mechanism for transferring file descriptors between cooperating processes: int sendfd(pid_t pid, int sig, int fd) Given a target process pid, the sendfd() syscall will create a duplicate file descriptor in a target task's (referred by pid) file table pointing to the file references by descriptor fd. Then, it will attempt to notify the target task by issuing a Posix.1b real-time signal (sig), carrying the new file descriptor as integer payload. If real-time signal can not be enqueued at the destination signal queue, the newly created file descriptor will be promptly closed. It is believed, that proposed sendfd() syscall, together with recently accepted memfd facility may greatly simplify development of parallel processing applications, by eliminating the need to rely on tricky and possibly insecure approaches involving domain sockets and such. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] fs: introduce sendfd() syscall
Present patch introduces exceptionally easy to use, low latency and low overhead mechanism for transferring file descriptors between cooperating processes: int sendfd(pid_t pid, int sig, int fd) Given a target process pid, the sendfd() syscall will create a duplicate file descriptor in a target task's (referred by pid) file table pointing to the file references by descriptor fd. Then, it will attempt to notify the target task by issuing a Posix.1b real-time signal (sig), carrying the new file descriptor as integer payload. If real-time signal can not be enqueued at the destination signal queue, the newly created file descriptor will be promptly closed. Signed-off-by: Alex Dubov oa...@yahoo.com --- fs/Makefile | 1 + fs/sendfd.c | 82 init/Kconfig | 11 3 files changed, 94 insertions(+) create mode 100644 fs/sendfd.c diff --git a/fs/Makefile b/fs/Makefile index da0bbb4..bed05a8 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_ANON_INODES) += anon_inodes.o obj-$(CONFIG_SIGNALFD) += signalfd.o obj-$(CONFIG_TIMERFD) += timerfd.o obj-$(CONFIG_EVENTFD) += eventfd.o +obj-$(CONFIG_SENDFD) += sendfd.o obj-$(CONFIG_AIO) += aio.o obj-$(CONFIG_FILE_LOCKING) += locks.o obj-$(CONFIG_COMPAT) += compat.o compat_ioctl.o diff --git a/fs/sendfd.c b/fs/sendfd.c new file mode 100644 index 000..1e85484 --- /dev/null +++ b/fs/sendfd.c @@ -0,0 +1,82 @@ +/* + * fs/sendfd.c + * + * Copyright (C) 2014 Alex Dubov oa...@yahoo.com + * + */ + +#include linux/file.h +#include linux/fdtable.h +#include linux/syscalls.h + +SYSCALL_DEFINE3(sendfd, pid_t, pid, int, sig, int, fd) +{ + struct siginfo s_info = { + .si_signo = sig, + .si_errno = 0, + .si_code = __SI_RT + }; + struct file *src_file = NULL; + struct task_struct *dst_task = NULL; + struct files_struct *dst_files = NULL; + unsigned long rlim = 0; + unsigned long flags = 0; + int rc = 0; + + if ((sig SIGRTMIN) || (sig SIGRTMAX)) + return -EINVAL; + + s_info.si_pid = task_pid_vnr(current); + s_info.si_uid = from_kuid_munged(current_user_ns(), current_uid()); + s_info.si_int = -1; + + src_file = fget(fd); + if (!src_file) + return -EBADF; + + rcu_read_lock(); + dst_task = find_task_by_vpid(pid); + + if (!dst_task) { + rc = -ESRCH; + goto out_put_src_file; + } + get_task_struct(dst_task); + rcu_read_unlock(); + + dst_files = get_files_struct(dst_task); + if (!dst_files) { + rc = -EMFILE; + goto out_put_dst_task; + } + + if (!lock_task_sighand(dst_task, flags)) { + rc = -EMFILE; + goto out_put_dst_files; + } + + rlim = task_rlimit(dst_task, RLIMIT_NOFILE); + + unlock_task_sighand(dst_task, flags); + + rc = __alloc_fd(dst_task-files, 0, rlim, O_CLOEXEC); + if (rc 0) + goto out_put_dst_files; + + s_info.si_int = rc; + + get_file(src_file); + __fd_install(dst_files, rc, src_file); + rc = kill_pid_info(sig, s_info, task_pid(dst_task)); + + if (rc 0) + __close_fd(dst_files, s_info.si_int); + +out_put_dst_files: + put_files_struct(dst_files); +out_put_dst_task: + put_task_struct(dst_task); +out_put_src_file: + fput(src_file); + return rc; +} diff --git a/init/Kconfig b/init/Kconfig index 2081a4d..dfe8b6f 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1525,6 +1525,17 @@ config EVENTFD If unsure, say Y. +config SENDFD + bool Enable sendfd() system call if EXPERT + default y + help + Enable the sendfd() system call that allows rapid duplication + of file descriptor across process boundaries. The target process + will receive a duplicate file descriptor delivered with one of + Posix.1b real-time signals. + + If unsure, say Y. + # syscall, maps, verifier config BPF_SYSCALL bool Enable bpf() system call if EXPERT -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memstick: Fix memory leak in memstick_check() error path
Hi, In the good old times, when this driver was first written, device name used to be a fixed size array (of 32 chars, if I'm not mistaken) in the kobj struct, so there was no need to free it explicitly. Since than, somebody changed the name field to become a loose pointer, but it's not obvious how it is supposed to be handled these days. From: Larry Finger To: Catalin Marinas Cc: Alex Dubov ; Linux Kernel Mailing List ; Kay Sievers ; Greg Kroah-Hartman Sent: Monday, 7 October 2013 11:02 AM Subject: Re: [PATCH] memstick: Fix memory leak in memstick_check() error path On 10/04/2013 03:54 AM, Catalin Marinas wrote: > On 3 October 2013 22:13, Larry Finger wrote: >> diff --git a/drivers/memstick/core/memstick.c >> b/drivers/memstick/core/memstick.c >> index ffcb10a..0c73a45 100644 >> --- a/drivers/memstick/core/memstick.c >> +++ b/drivers/memstick/core/memstick.c >> @@ -415,6 +415,7 @@ static struct memstick_dev *memstick_alloc_card(struct >> memstick_host *host) >> return card; >> err_out: >> host->card = old_card; >> + kfree(card->dev.kobj.name); > > It looks weird to go into dev.kobj internals here for freeing the > name. There is also memstick_free_card() which doesn't seem to do > anything about the name freeing. > > Should memstick_alloc_card() do a device_initialise(>dev) and in > memstick_free_card() (or the error path) do a put_device(>dev)? > This should take care of kobj.name as well via kobject_put(). I tried several code changes that included adding a device_initialize() call, but all of them oopsed even when I followed the examples in other drivers. Adding a put_device() without the device_initialize() did not oops, but it still leaked the name. We could avoid going into the dev.kobj internals if a device_free_name() routine existed as a companion to dev_set_name(). Larry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memstick: Fix memory leak in memstick_check() error path
Hi, In the good old times, when this driver was first written, device name used to be a fixed size array (of 32 chars, if I'm not mistaken) in the kobj struct, so there was no need to free it explicitly. Since than, somebody changed the name field to become a loose pointer, but it's not obvious how it is supposed to be handled these days. From: Larry Finger larry.fin...@lwfinger.net To: Catalin Marinas catalin.mari...@arm.com Cc: Alex Dubov oa...@yahoo.com; Linux Kernel Mailing List linux-kernel@vger.kernel.org; Kay Sievers kay.siev...@vrfy.org; Greg Kroah-Hartman gre...@suse.de Sent: Monday, 7 October 2013 11:02 AM Subject: Re: [PATCH] memstick: Fix memory leak in memstick_check() error path On 10/04/2013 03:54 AM, Catalin Marinas wrote: On 3 October 2013 22:13, Larry Finger larry.fin...@lwfinger.net wrote: diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c index ffcb10a..0c73a45 100644 --- a/drivers/memstick/core/memstick.c +++ b/drivers/memstick/core/memstick.c @@ -415,6 +415,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host) return card; err_out: host-card = old_card; + kfree(card-dev.kobj.name); It looks weird to go into dev.kobj internals here for freeing the name. There is also memstick_free_card() which doesn't seem to do anything about the name freeing. Should memstick_alloc_card() do a device_initialise(card-dev) and in memstick_free_card() (or the error path) do a put_device(card-dev)? This should take care of kobj.name as well via kobject_put(). I tried several code changes that included adding a device_initialize() call, but all of them oopsed even when I followed the examples in other drivers. Adding a put_device() without the device_initialize() did not oops, but it still leaked the name. We could avoid going into the dev.kobj internals if a device_free_name() routine existed as a companion to dev_set_name(). Larry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 0/3] Add modules to support realtek PCIE card reader
> > > Hi Alex: Hi, > > Do you have any comment on the MEMSTICK part in this v7 patchset? Can > you help to merge the patch to the kernel tree? I'm afraid that presently I don't have much time to look at the memstick related stuff any further. Hopefully, somebody else can step in and take a look at your submission. Sorry for that. > > Thanks. > Wei WANG > Regards, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 0/3] Add modules to support realtek PCIE card reader
Hi Alex: Hi, Do you have any comment on the MEMSTICK part in this v7 patchset? Can you help to merge the patch to the kernel tree? I'm afraid that presently I don't have much time to look at the memstick related stuff any further. Hopefully, somebody else can step in and take a look at your submission. Sorry for that. Thanks. Wei WANG Regards, Alex -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/memstick/host/tifm_ms.c breakage
--- Al Viro <[EMAIL PROTECTED]> wrote: > readl(sock + ...) that should've been readl(sock->addr + ...) > Thanks. It's a first member in struct, so the problem was just sitting there unnoticed. Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/memstick/host/tifm_ms.c breakage
--- Al Viro [EMAIL PROTECTED] wrote: readl(sock + ...) that should've been readl(sock-addr + ...) Thanks. It's a first member in struct, so the problem was just sitting there unnoticed. Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] memstick: fix attribute structure casting in mspro_block_resume
Signed-off-by: Alex Dubov <[EMAIL PROTECTED]> --- drivers/memstick/core/mspro_block.c | 10 +++--- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c index b9bd0aa..423ad8c 100644 --- a/drivers/memstick/core/mspro_block.c +++ b/drivers/memstick/core/mspro_block.c @@ -1237,7 +1237,7 @@ static int mspro_block_resume(struct memstick_dev *card) struct mspro_block_data *new_msb; struct memstick_host *host = card->host; - struct mspro_sys_attr s_attr, r_attr; + struct mspro_sys_attr *s_attr, *r_attr; unsigned char cnt; mutex_lock(>lock); @@ -1254,12 +1254,8 @@ static int mspro_block_resume(struct memstick_dev *card) for (cnt = 0; new_msb->attr_group.attrs[cnt] && msb->attr_group.attrs[cnt]; ++cnt) { - s_attr = container_of(new_msb->attr_group.attrs[cnt], - struct mspro_sys_attr, - dev_attr); - r_attr = container_of(msb->attr_group.attrs[cnt], - struct mspro_sys_attr, - dev_attr); + s_attr = mspro_from_sysfs_attr(new_msb->attr_group.attrs[cnt]); + r_attr = mspro_from_sysfs_attr(msb->attr_group.attrs[cnt]); if (s_attr->id == MSPRO_BLOCK_ID_SYSINFO && r_attr->id == s_attr->id) { -- 1.5.3.6 Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] memstick: fix attribute structure casting in mspro_block_resume
Signed-off-by: Alex Dubov [EMAIL PROTECTED] --- drivers/memstick/core/mspro_block.c | 10 +++--- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c index b9bd0aa..423ad8c 100644 --- a/drivers/memstick/core/mspro_block.c +++ b/drivers/memstick/core/mspro_block.c @@ -1237,7 +1237,7 @@ static int mspro_block_resume(struct memstick_dev *card) struct mspro_block_data *new_msb; struct memstick_host *host = card-host; - struct mspro_sys_attr s_attr, r_attr; + struct mspro_sys_attr *s_attr, *r_attr; unsigned char cnt; mutex_lock(host-lock); @@ -1254,12 +1254,8 @@ static int mspro_block_resume(struct memstick_dev *card) for (cnt = 0; new_msb-attr_group.attrs[cnt] msb-attr_group.attrs[cnt]; ++cnt) { - s_attr = container_of(new_msb-attr_group.attrs[cnt], - struct mspro_sys_attr, - dev_attr); - r_attr = container_of(msb-attr_group.attrs[cnt], - struct mspro_sys_attr, - dev_attr); + s_attr = mspro_from_sysfs_attr(new_msb-attr_group.attrs[cnt]); + r_attr = mspro_from_sysfs_attr(msb-attr_group.attrs[cnt]); if (s_attr-id == MSPRO_BLOCK_ID_SYSINFO r_attr-id == s_attr-id) { -- 1.5.3.6 Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] memstick: use __blk_end_request to complete requests
Signed-off-by: Alex Dubov <[EMAIL PROTECTED]> --- mspro_block.c.orig 2008-02-04 15:25:16.0 +1100 +++ mspro_block.c 2008-02-04 15:26:28.226886699 +1100 @@ -668,20 +668,13 @@ spin_lock_irqsave(>q_lock, flags); if (rc >= 0) - chunk = end_that_request_chunk(req, 1, rc); + chunk = __blk_end_request(req, 0, rc); else - chunk = end_that_request_first(req, rc, - req->current_nr_sectors); + chunk = __blk_end_request(req, rc, 0); dev_dbg(>dev, "end chunk %d, %d\n", rc, chunk); - if (!chunk) { - add_disk_randomness(req->rq_disk); - blkdev_dequeue_request(req); - end_that_request_last(req, rc > 0 ? 1 : rc); - } spin_unlock_irqrestore(>q_lock, flags); } while (chunk); - } static int mspro_block_has_request(struct mspro_block_data *msb) Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] memstick: use __blk_end_request to complete requests
Signed-off-by: Alex Dubov [EMAIL PROTECTED] --- mspro_block.c.orig 2008-02-04 15:25:16.0 +1100 +++ mspro_block.c 2008-02-04 15:26:28.226886699 +1100 @@ -668,20 +668,13 @@ spin_lock_irqsave(msb-q_lock, flags); if (rc = 0) - chunk = end_that_request_chunk(req, 1, rc); + chunk = __blk_end_request(req, 0, rc); else - chunk = end_that_request_first(req, rc, - req-current_nr_sectors); + chunk = __blk_end_request(req, rc, 0); dev_dbg(card-dev, end chunk %d, %d\n, rc, chunk); - if (!chunk) { - add_disk_randomness(req-rq_disk); - blkdev_dequeue_request(req); - end_that_request_last(req, rc 0 ? 1 : rc); - } spin_unlock_irqrestore(msb-q_lock, flags); } while (chunk); - } static int mspro_block_has_request(struct mspro_block_data *msb) Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] [MEMSTICK] Updates for the memstick driver
* Mark shared inline functions as static * Use member-at-a-time assignment for protocol structures * Comments for publicly exported functions * Use end_queued_request to end unhandled block layer requests * Use sysfs attribute group to export MSPro attributes * Fix includes * Use scnprintf instead of snprintf where string length matters * Remove spurious get_device/put_device in probe method diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c index 46e5f9b..bba467f 100644 --- a/drivers/memstick/core/memstick.c +++ b/drivers/memstick/core/memstick.c @@ -12,11 +12,10 @@ * */ -#include #include #include -#include #include +#include #define DRIVER_NAME "memstick" #define DRIVER_VERSION "0.2" @@ -86,13 +85,11 @@ static int memstick_device_probe(struct device *dev) driver); int rc = -ENODEV; - get_device(dev); if (dev->driver && drv->probe) { rc = drv->probe(card); if (!rc) - return 0; + get_device(dev); } - put_device(dev); return rc; } @@ -205,12 +202,26 @@ static int memstick_dummy_check(struct memstick_dev *card) return 0; } +/** + * memstick_detect_change - schedule media detection on memstick host + * @host - host to use + */ void memstick_detect_change(struct memstick_host *host) { queue_work(workqueue, >media_checker); } EXPORT_SYMBOL(memstick_detect_change); +/** + * memstick_next_req - called by host driver to obtain next request to process + * @host - host to use + * @mrq - pointer to stick the request to + * + * Host calls this function from idle state (*mrq == NULL) or after finishing + * previous request (*mrq should point to it). If previous request was + * unsuccessful, it is retried for predetermined number of times. Return value + * of 0 means that new request was assigned to the host. + */ int memstick_next_req(struct memstick_host *host, struct memstick_request **mrq) { int rc = -ENXIO; @@ -233,6 +244,10 @@ int memstick_next_req(struct memstick_host *host, struct memstick_request **mrq) } EXPORT_SYMBOL(memstick_next_req); +/** + * memstick_new_req - notify the host that some requests are pending + * @host - host to use + */ void memstick_new_req(struct memstick_host *host) { host->retries = cmd_retries; @@ -240,6 +255,12 @@ void memstick_new_req(struct memstick_host *host) } EXPORT_SYMBOL(memstick_new_req); +/** + * memstick_init_req_sg - set request fields needed for bulk data transfer + * @mrq - request to use + * @tpc - memstick Transport Protocol Command + * @sg - TPC argument + */ void memstick_init_req_sg(struct memstick_request *mrq, unsigned char tpc, struct scatterlist *sg) { @@ -261,6 +282,17 @@ void memstick_init_req_sg(struct memstick_request *mrq, unsigned char tpc, } EXPORT_SYMBOL(memstick_init_req_sg); +/** + * memstick_init_req - set request fields needed for short data transfer + * @mrq - request to use + * @tpc - memstick Transport Protocol Command + * @buf - TPC argument buffer + * @length - TPC argument size + * + * The intended use of this function (transfer of data items several bytes + * in size) allows us to just copy the value between request structure and + * user supplied buffer. + */ void memstick_init_req(struct memstick_request *mrq, unsigned char tpc, void *buf, size_t length) { @@ -285,6 +317,13 @@ void memstick_init_req(struct memstick_request *mrq, unsigned char tpc, } EXPORT_SYMBOL(memstick_init_req); +/* + * Functions prefixed with "h_" are protocol callbacks. They can be called from + * interrupt context. Return value of 0 means that request processing is still + * ongoing, while special error value of -EAGAIN means that current request is + * finished (and request processor should come back some time later). + */ + static int h_memstick_read_dev_id(struct memstick_dev *card, struct memstick_request **mrq) { @@ -298,12 +337,10 @@ static int h_memstick_read_dev_id(struct memstick_dev *card, } else { if (!(*mrq)->error) { memcpy(_reg, (*mrq)->data, sizeof(id_reg)); - card->id = (struct memstick_device_id){ - .match_flags = MEMSTICK_MATCH_ALL, - .type = id_reg.type, - .category = id_reg.category, - .class = id_reg.class - }; + card->id.match_flags = MEMSTICK_MATCH_ALL; + card->id.type = id_reg.type; + card->id.category = id_reg.category; + card->id.class = id_reg.class; } complete(>mrq_complete); return -EAGAIN; @@ -325,6 +362,11 @@ static int
[PATCH] [MEMSTICK] Updates for the memstick driver
* Mark shared inline functions as static * Use member-at-a-time assignment for protocol structures * Comments for publicly exported functions * Use end_queued_request to end unhandled block layer requests * Use sysfs attribute group to export MSPro attributes * Fix includes * Use scnprintf instead of snprintf where string length matters * Remove spurious get_device/put_device in probe method diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c index 46e5f9b..bba467f 100644 --- a/drivers/memstick/core/memstick.c +++ b/drivers/memstick/core/memstick.c @@ -12,11 +12,10 @@ * */ -#include linux/tifm.h #include linux/memstick.h #include linux/idr.h -#include linux/scatterlist.h #include linux/fs.h +#include linux/delay.h #define DRIVER_NAME memstick #define DRIVER_VERSION 0.2 @@ -86,13 +85,11 @@ static int memstick_device_probe(struct device *dev) driver); int rc = -ENODEV; - get_device(dev); if (dev-driver drv-probe) { rc = drv-probe(card); if (!rc) - return 0; + get_device(dev); } - put_device(dev); return rc; } @@ -205,12 +202,26 @@ static int memstick_dummy_check(struct memstick_dev *card) return 0; } +/** + * memstick_detect_change - schedule media detection on memstick host + * @host - host to use + */ void memstick_detect_change(struct memstick_host *host) { queue_work(workqueue, host-media_checker); } EXPORT_SYMBOL(memstick_detect_change); +/** + * memstick_next_req - called by host driver to obtain next request to process + * @host - host to use + * @mrq - pointer to stick the request to + * + * Host calls this function from idle state (*mrq == NULL) or after finishing + * previous request (*mrq should point to it). If previous request was + * unsuccessful, it is retried for predetermined number of times. Return value + * of 0 means that new request was assigned to the host. + */ int memstick_next_req(struct memstick_host *host, struct memstick_request **mrq) { int rc = -ENXIO; @@ -233,6 +244,10 @@ int memstick_next_req(struct memstick_host *host, struct memstick_request **mrq) } EXPORT_SYMBOL(memstick_next_req); +/** + * memstick_new_req - notify the host that some requests are pending + * @host - host to use + */ void memstick_new_req(struct memstick_host *host) { host-retries = cmd_retries; @@ -240,6 +255,12 @@ void memstick_new_req(struct memstick_host *host) } EXPORT_SYMBOL(memstick_new_req); +/** + * memstick_init_req_sg - set request fields needed for bulk data transfer + * @mrq - request to use + * @tpc - memstick Transport Protocol Command + * @sg - TPC argument + */ void memstick_init_req_sg(struct memstick_request *mrq, unsigned char tpc, struct scatterlist *sg) { @@ -261,6 +282,17 @@ void memstick_init_req_sg(struct memstick_request *mrq, unsigned char tpc, } EXPORT_SYMBOL(memstick_init_req_sg); +/** + * memstick_init_req - set request fields needed for short data transfer + * @mrq - request to use + * @tpc - memstick Transport Protocol Command + * @buf - TPC argument buffer + * @length - TPC argument size + * + * The intended use of this function (transfer of data items several bytes + * in size) allows us to just copy the value between request structure and + * user supplied buffer. + */ void memstick_init_req(struct memstick_request *mrq, unsigned char tpc, void *buf, size_t length) { @@ -285,6 +317,13 @@ void memstick_init_req(struct memstick_request *mrq, unsigned char tpc, } EXPORT_SYMBOL(memstick_init_req); +/* + * Functions prefixed with h_ are protocol callbacks. They can be called from + * interrupt context. Return value of 0 means that request processing is still + * ongoing, while special error value of -EAGAIN means that current request is + * finished (and request processor should come back some time later). + */ + static int h_memstick_read_dev_id(struct memstick_dev *card, struct memstick_request **mrq) { @@ -298,12 +337,10 @@ static int h_memstick_read_dev_id(struct memstick_dev *card, } else { if (!(*mrq)-error) { memcpy(id_reg, (*mrq)-data, sizeof(id_reg)); - card-id = (struct memstick_device_id){ - .match_flags = MEMSTICK_MATCH_ALL, - .type = id_reg.type, - .category = id_reg.category, - .class = id_reg.class - }; + card-id.match_flags = MEMSTICK_MATCH_ALL; + card-id.type = id_reg.type; + card-id.category = id_reg.category; + card-id.class = id_reg.class; }
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Andrew Morton <[EMAIL PROTECTED]> wrote: > On Wed, 2 Jan 2008 17:42:24 +1100 [EMAIL PROTECTED] wrote: > > > From: Alex Dubov <[EMAIL PROTECTED]> > > > > Sony MemoryStick cards are used in many products manufactured by Sony. They > > are available both as storage and as IO expansion cards. Currently, only > > MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick > > interface. > > > > Will you be running a git tree for this? If so, please send me the link. > I rectified several additional issues with the driver. I hope you can pull the changes from here: http://58.96.64.63/~oakad/git/.git memstick Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Andrew Morton [EMAIL PROTECTED] wrote: On Wed, 2 Jan 2008 17:42:24 +1100 [EMAIL PROTECTED] wrote: From: Alex Dubov [EMAIL PROTECTED] Sony MemoryStick cards are used in many products manufactured by Sony. They are available both as storage and as IO expansion cards. Currently, only MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick interface. Will you be running a git tree for this? If so, please send me the link. I rectified several additional issues with the driver. I hope you can pull the changes from here: http://58.96.64.63/~oakad/git/.git memstick Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Mariusz Kozlowski <[EMAIL PROTECTED]> wrote: > Hello, > > > Sony MemoryStick cards are used in many products manufactured by Sony. They > > are available both as storage and as IO expansion cards. Currently, only > > MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick > > interface. > > I tried it here and it doesn't work. My Vaio (PCG-FR285M) is from ~2003 (Is > it too old > for this?). I have some memory stick cards around so If you want a tester > just drop me > an email. > > Regards, > > Mariusz > The build year is nowhere as helpful as 'lspci -vv' output. Then, given that your vaio is equipped with tifm controller, you'll have to build the driver with debugging enabled and send me the relevant excerpt of your system log. You should have the following modules loaded, by the way: memstick mspro_block tifm_core tifm_7xx1 tifm_ms The autoloading is handled via udev (so the relevant rules are not there yet). Never miss a thing. Make Yahoo your home page. http://www.yahoo.com/r/hs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Mariusz Kozlowski [EMAIL PROTECTED] wrote: Hello, Sony MemoryStick cards are used in many products manufactured by Sony. They are available both as storage and as IO expansion cards. Currently, only MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick interface. I tried it here and it doesn't work. My Vaio (PCG-FR285M) is from ~2003 (Is it too old for this?). I have some memory stick cards around so If you want a tester just drop me an email. Regards, Mariusz The build year is nowhere as helpful as 'lspci -vv' output. Then, given that your vaio is equipped with tifm controller, you'll have to build the driver with debugging enabled and send me the relevant excerpt of your system log. You should have the following modules loaded, by the way: memstick mspro_block tifm_core tifm_7xx1 tifm_ms The autoloading is handled via udev (so the relevant rules are not there yet). Never miss a thing. Make Yahoo your home page. http://www.yahoo.com/r/hs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Andrew Morton <[EMAIL PROTECTED]> wrote: > On Wed, 2 Jan 2008 17:42:24 +1100 [EMAIL PROTECTED] wrote: > > > From: Alex Dubov <[EMAIL PROTECTED]> > > > > Sony MemoryStick cards are used in many products manufactured by Sony. They > > are available both as storage and as IO expansion cards. Currently, only > > MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick > > interface. > > > > Will you be running a git tree for this? If so, please send me the link. > > Will this enable that thus-far useless slot on my Vaio? > > Where did the info come from which enabled this driver to be written? I > thought Sony were super-secretive about this stuff? > I'm going to set-up git tree, yes. It should support some vaios (newer ones for sure), as far as I know. The primary reference for the driver is this one: http://zeniv.linux.org.uk/~winbond/ (link is somewhat slow, but works). Winbond developed GPLv2 driver for linux and there was enough info in their source code for my implementation (totally different from their's). Some reverse engineering of the windows driver was needed too (for the TI registers and such). > > > @@ -0,0 +1,26 @@ > > +# > > +# MemoryStick subsystem configuration > > +# > > + > > +menuconfig MEMSTICK > > + tristate "Sony MemoryStick card support (EXPERIMENTAL)" > > + help > > + Sony MemoryStick is a proprietary storage/extension card protocol. > > + > > + If you want MemoryStick support, you should say Y here and also > > + to the specific driver for your MMC interface. > > Are you sure this has enough dependencies? CONFIG_TIFM_* for a start? This is supposed to be a generic layer, akin to mmc. I have a jmicron backend in the works, and windbond backend will be possible, too. > > > > We can create a config which has CONFIG_TIFM_CORE=m, CONFIG_MEMSTICK=y. > That might not work? > > Anyway, please have a think about that. Considering the previous point this should be ok. > > > ... > > > > + > > +struct mspro_sys_attr { > > + size_t size; > > + unsigned char *data; > > + unsigned char id; > > + charname[32]; > > + struct device_attribute sys_attr; > > +}; > > + > > +struct mspro_attr_entry { > > + unsigned int address; > > + unsigned int size; > > + unsigned char id; > > + unsigned char reserved[3]; > > +} __attribute__((packed)); > > + > > +struct mspro_attribute { > > + unsigned short signature; > > + unsigned short version; > > + unsigned char count; > > + unsigned char reserved[11]; > > + struct mspro_attr_entry entries[]; > > +} __attribute__((packed)); > > Why are these packed? Do they map onto something whcih hardware knows > about? Yes, of course; all structures with "packed" attribute correspond to appropriate protocol ones (the endianness is tweaked as needed - most, but not all fields, are big-endian). > > > > ... > > > > +struct mspro_block_data { > > + struct memstick_dev *card; > > + unsigned int usage_count; > > + struct gendisk*disk; > > + struct request_queue *queue; > > + spinlock_tq_lock; > > + wait_queue_head_t q_wait; > > + struct task_struct*q_thread; > > + > > + unsigned shortpage_size; > > + unsigned shortcylinders; > > + unsigned shortheads; > > + unsigned shortsectors_per_track; > > + > > + unsigned char system; > > + unsigned char read_only:1, > > + active:1, > > + has_request:1, > > + data_dir:1; > > You're aware that these four fields occupy the same machine word and that > the compiler provides no locking? So if one thread attempts to modify > "read_only" while another modifies "has_request", one can corrupt the work > of the other? > > If this is indeed safe then a comment here whcih clearly explains that > would be useful. The access to these fields should be exclusively under q_lock (I'll check the locking again, just to make sure). After all, the same applies to old fashioned bit masks. > > > +} > > + > > +static ssize_t mspro_block_attr_show_modelname(struct device *dev, > > + struct device_attribute *attr, > > + char *buf
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Andrew Morton [EMAIL PROTECTED] wrote: On Wed, 2 Jan 2008 17:42:24 +1100 [EMAIL PROTECTED] wrote: From: Alex Dubov [EMAIL PROTECTED] Sony MemoryStick cards are used in many products manufactured by Sony. They are available both as storage and as IO expansion cards. Currently, only MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick interface. Will you be running a git tree for this? If so, please send me the link. Will this enable that thus-far useless slot on my Vaio? Where did the info come from which enabled this driver to be written? I thought Sony were super-secretive about this stuff? I'm going to set-up git tree, yes. It should support some vaios (newer ones for sure), as far as I know. The primary reference for the driver is this one: http://zeniv.linux.org.uk/~winbond/ (link is somewhat slow, but works). Winbond developed GPLv2 driver for linux and there was enough info in their source code for my implementation (totally different from their's). Some reverse engineering of the windows driver was needed too (for the TI registers and such). @@ -0,0 +1,26 @@ +# +# MemoryStick subsystem configuration +# + +menuconfig MEMSTICK + tristate Sony MemoryStick card support (EXPERIMENTAL) + help + Sony MemoryStick is a proprietary storage/extension card protocol. + + If you want MemoryStick support, you should say Y here and also + to the specific driver for your MMC interface. Are you sure this has enough dependencies? CONFIG_TIFM_* for a start? This is supposed to be a generic layer, akin to mmc. I have a jmicron backend in the works, and windbond backend will be possible, too. fiddles with Kconfig We can create a config which has CONFIG_TIFM_CORE=m, CONFIG_MEMSTICK=y. That might not work? Anyway, please have a think about that. Considering the previous point this should be ok. ... + +struct mspro_sys_attr { + size_t size; + unsigned char *data; + unsigned char id; + charname[32]; + struct device_attribute sys_attr; +}; + +struct mspro_attr_entry { + unsigned int address; + unsigned int size; + unsigned char id; + unsigned char reserved[3]; +} __attribute__((packed)); + +struct mspro_attribute { + unsigned short signature; + unsigned short version; + unsigned char count; + unsigned char reserved[11]; + struct mspro_attr_entry entries[]; +} __attribute__((packed)); Why are these packed? Do they map onto something whcih hardware knows about? Yes, of course; all structures with packed attribute correspond to appropriate protocol ones (the endianness is tweaked as needed - most, but not all fields, are big-endian). ... +struct mspro_block_data { + struct memstick_dev *card; + unsigned int usage_count; + struct gendisk*disk; + struct request_queue *queue; + spinlock_tq_lock; + wait_queue_head_t q_wait; + struct task_struct*q_thread; + + unsigned shortpage_size; + unsigned shortcylinders; + unsigned shortheads; + unsigned shortsectors_per_track; + + unsigned char system; + unsigned char read_only:1, + active:1, + has_request:1, + data_dir:1; You're aware that these four fields occupy the same machine word and that the compiler provides no locking? So if one thread attempts to modify read_only while another modifies has_request, one can corrupt the work of the other? If this is indeed safe then a comment here whcih clearly explains that would be useful. The access to these fields should be exclusively under q_lock (I'll check the locking again, just to make sure). After all, the same applies to old fashioned bit masks. +} + +static ssize_t mspro_block_attr_show_modelname(struct device *dev, + struct device_attribute *attr, + char *buffer) +{ + struct mspro_sys_attr *x_attr = container_of(attr, +struct mspro_sys_attr, +sys_attr); + + return snprintf(buffer, PAGE_SIZE, %s, x_attr-data); +} ... +static int mspro_block_sysfs_register(struct memstick_dev *card) +{ + struct mspro_block_data *msb = memstick_get_drvdata(card); + int cnt, rc = 0; + + for (cnt = 0; cnt msb-attr_count; cnt++) { + rc = device_create_file(card-dev, + msb-attributes[cnt].sys_attr); + + if (rc) { + if (cnt) { + for (cnt--; cnt = 0; cnt--) ow. my brain
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Carlos Corbacho <[EMAIL PROTECTED]> wrote: > > So do we require changes to the userspace udev rules here, or just some use > of > modalias in the drivers? > It was handled by whoever manages the distro's udev rules until now. Here's the example: https://lists.ubuntu.com/archives/ubuntu-patches/2007-September/012537.html Never miss a thing. Make Yahoo your home page. http://www.yahoo.com/r/hs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Jan Engelhardt <[EMAIL PROTECTED]> wrote: > > On Dec 31 2007 00:01, Carlos Corbacho wrote: > >On Monday 24 December 2007 03:06:37 [EMAIL PROTECTED] wrote: > >> From: Alex Dubov <[EMAIL PROTECTED]> > >> > >> Sony MemoryStick cards are used in many products manufactured by Sony. They > >> are available both as storage and as IO expansion cards. Currently, only > >> MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick > >> interface. > > Actually... my MS slot on PCG-U3 is recognized through usb_storage. > USB storage type controllers have firmware that translates native MS protocol into USB storage (scsi-like) one. We are talking here about native protocol support, needed for dumb controllers. Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
> However, my only concerns are that: > > 1) tifm_ms was not autoloaded > 2) On loading tifm_ms, only memstick was autoloaded - mspro_block was not. > > Although, whether this is an issue with userspace (ie. udev) not dealing with > the modules properly, I don't know. > This is exactly the same as with tifm_sd module if you noticed. First, there are no modaliases for tifm (I was thinking to add them later, when at least two card types are supported, if at all), so the appropriate udev rule should be added (similar to the sd one). Second, it is impossible to guess in advance, which type of card is put into slot (Pro, legacy or IO of either sort). Same as mmc_block, actually. The only way to get it autoloaded, is, again, with appropriate udev rule (uevent is emitted when card type is detected). Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Jan Engelhardt [EMAIL PROTECTED] wrote: On Dec 31 2007 00:01, Carlos Corbacho wrote: On Monday 24 December 2007 03:06:37 [EMAIL PROTECTED] wrote: From: Alex Dubov [EMAIL PROTECTED] Sony MemoryStick cards are used in many products manufactured by Sony. They are available both as storage and as IO expansion cards. Currently, only MemoryStick Pro storage cards are supported via TI FlashMedia MemoryStick interface. Actually... my MS slot on PCG-U3 is recognized through usb_storage. USB storage type controllers have firmware that translates native MS protocol into USB storage (scsi-like) one. We are talking here about native protocol support, needed for dumb controllers. Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
However, my only concerns are that: 1) tifm_ms was not autoloaded 2) On loading tifm_ms, only memstick was autoloaded - mspro_block was not. Although, whether this is an issue with userspace (ie. udev) not dealing with the modules properly, I don't know. This is exactly the same as with tifm_sd module if you noticed. First, there are no modaliases for tifm (I was thinking to add them later, when at least two card types are supported, if at all), so the appropriate udev rule should be added (similar to the sd one). Second, it is impossible to guess in advance, which type of card is put into slot (Pro, legacy or IO of either sort). Same as mmc_block, actually. The only way to get it autoloaded, is, again, with appropriate udev rule (uevent is emitted when card type is detected). Looking for last minute shopping deals? Find them fast with Yahoo! Search. http://tools.search.yahoo.com/newsearch/category.php?category=shopping -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [MEMSTICK] Initial commit for Sony MemoryStick support
--- Carlos Corbacho [EMAIL PROTECTED] wrote: So do we require changes to the userspace udev rules here, or just some use of modalias in the drivers? It was handled by whoever manages the distro's udev rules until now. Here's the example: https://lists.ubuntu.com/archives/ubuntu-patches/2007-September/012537.html Never miss a thing. Make Yahoo your home page. http://www.yahoo.com/r/hs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
MemoryStick / Pro support
After a much longer, than expected, time I managed to implement a support for MemoryStick (read-only currently, as there's still a subtle data corruption bug with writes) and MemoryStick Pro cards. The implementation follows the MMC driver model (there exist MSIO cards, but none are supported at the moment). The MS Pro support appears stable from what I can learn from user reports. Nevertheless, I've implemented a couple of diagnostics files in the "sys" filesystem, as well as low level format facility for legacy MS cards. Currently only TI Flashmedia adapters are supported, but I'm working on a JMicron JMB38x adapter support and I know for sure that it'll be easy to support a Winbond 528 adapter, as I used its GPLed driver as a reference for a more generic implementation. I would like to get an advice on the way to arrange the files in the kernel tree. My current idea is: memstick.h-> include/linux memstick.c-> drivers/memstick ("bus" support) ms_block.c-> drivers/memstick (legacy MS storage support) mspro_block.c -> drivers/memstick (MS Pro storage support) tifm_ms.c -> drivers/memstick (TI Flashmedia low level driver) I also wonder, where do I send the patches if nobody currently maintains this thing? __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
MemoryStick / Pro support
After a much longer, than expected, time I managed to implement a support for MemoryStick (read-only currently, as there's still a subtle data corruption bug with writes) and MemoryStick Pro cards. The implementation follows the MMC driver model (there exist MSIO cards, but none are supported at the moment). The MS Pro support appears stable from what I can learn from user reports. Nevertheless, I've implemented a couple of diagnostics files in the sys filesystem, as well as low level format facility for legacy MS cards. Currently only TI Flashmedia adapters are supported, but I'm working on a JMicron JMB38x adapter support and I know for sure that it'll be easy to support a Winbond 528 adapter, as I used its GPLed driver as a reference for a more generic implementation. I would like to get an advice on the way to arrange the files in the kernel tree. My current idea is: memstick.h- include/linux memstick.c- drivers/memstick (bus support) ms_block.c- drivers/memstick (legacy MS storage support) mspro_block.c - drivers/memstick (MS Pro storage support) tifm_ms.c - drivers/memstick (TI Flashmedia low level driver) I also wonder, where do I send the patches if nobody currently maintains this thing? __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: idio{,ma}tic typos (was Re: + fix-vm_can_nonlinear-check-in-sys_remap_file_pages.patch added to -mm tree)
--- Pierre Ossman <[EMAIL PROTECTED]> wrote: > On Wed, 10 Oct 2007 14:45:40 +0400 > Alexey Dobriyan <[EMAIL PROTECTED]> wrote: > > > ["if (!x & y)" patch from [EMAIL PROTECTED] > > ["if (!x & y)" patch from [EMAIL PROTECTED] > > ["if (!x & y)" patches from [EMAIL PROTECTED] > > > > While we're at it, below is somewhat ugly sparse patch for detecting > > "&& 0x" typos. > > > > The maintainer for tifm is Alex Dubov, so cc:ing him. > > > drivers/mmc/host/tifm_sd.c:183:9: warning: dubious && 0x > > > > if ((r_data->flags & MMC_DATA_WRITE) > > && DATA_CARRY) > > writel(host->bounce_buf_data[0], > > host->dev->addr > > + SOCK_MMCSD_DATA); > > > > given that DATA_CARRY is always used together with > > ->cmd_flags, this place is asking for obvious fixlet: > > > > > > [PATCH] tifm_sd.c: fix DATA_CARRY check > > > > Signed-off-by: Alexey Dobriyan <[EMAIL PROTECTED]> > > --- > > > > drivers/mmc/host/tifm_sd.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > --- a/drivers/mmc/host/tifm_sd.c > > +++ b/drivers/mmc/host/tifm_sd.c > > @@ -180,7 +180,7 @@ static void tifm_sd_transfer_data(struct tifm_sd > > *host) host->sg_pos++; > > if (host->sg_pos == host->sg_len) { > > if ((r_data->flags & MMC_DATA_WRITE) > > - && DATA_CARRY) > > + && (host->cmd_flags & > > DATA_CARRY)) writel(host->bounce_buf_data[0], > >host->dev->addr > >+ SOCK_MMCSD_DATA); > > > > > > > > Rgds > Pierre > Oops. I wonder why this was never triggered (some users are having problems with dma, so they use PIO rather extensively). The patch is probably correct, but I can't do any testing because I'm currently on vacation. Shape Yahoo! in your own image. Join our Network Research Panel today! http://surveylink.yahoo.com/gmrs/yahoo_panel_invite.asp?a=7 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: idio{,ma}tic typos (was Re: + fix-vm_can_nonlinear-check-in-sys_remap_file_pages.patch added to -mm tree)
--- Pierre Ossman [EMAIL PROTECTED] wrote: On Wed, 10 Oct 2007 14:45:40 +0400 Alexey Dobriyan [EMAIL PROTECTED] wrote: [if (!x y) patch from [EMAIL PROTECTED] [if (!x y) patch from [EMAIL PROTECTED] [if (!x y) patches from [EMAIL PROTECTED] While we're at it, below is somewhat ugly sparse patch for detecting 0x typos. The maintainer for tifm is Alex Dubov, so cc:ing him. drivers/mmc/host/tifm_sd.c:183:9: warning: dubious 0x if ((r_data-flags MMC_DATA_WRITE) DATA_CARRY) writel(host-bounce_buf_data[0], host-dev-addr + SOCK_MMCSD_DATA); given that DATA_CARRY is always used together with -cmd_flags, this place is asking for obvious fixlet: [PATCH] tifm_sd.c: fix DATA_CARRY check Signed-off-by: Alexey Dobriyan [EMAIL PROTECTED] --- drivers/mmc/host/tifm_sd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/mmc/host/tifm_sd.c +++ b/drivers/mmc/host/tifm_sd.c @@ -180,7 +180,7 @@ static void tifm_sd_transfer_data(struct tifm_sd *host) host-sg_pos++; if (host-sg_pos == host-sg_len) { if ((r_data-flags MMC_DATA_WRITE) -DATA_CARRY) +(host-cmd_flags DATA_CARRY)) writel(host-bounce_buf_data[0], host-dev-addr + SOCK_MMCSD_DATA); Rgds Pierre Oops. I wonder why this was never triggered (some users are having problems with dma, so they use PIO rather extensively). The patch is probably correct, but I can't do any testing because I'm currently on vacation. Shape Yahoo! in your own image. Join our Network Research Panel today! http://surveylink.yahoo.com/gmrs/yahoo_panel_invite.asp?a=7 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [tifm] Infinite loop
--- "Renato S. Yamane" <[EMAIL PROTECTED]> wrote: > When I insert a SDCard in my laptop M45-S355 my system crash because > tifm start a infinite loop. See below more detail about SD/MMC Card and > infinite loop. If you are using the built-in version found in 2.6.21 then it is a known problem. You can use the driver from here, to fix this: https://developer.berlios.de/project/showfiles.php?group_id=5510 (or wait for 2.6.22) Luggage? GPS? Comic books? Check out fitting gifts for grads at Yahoo! Search http://search.yahoo.com/search?fr=oni_on_mail=graduation+gifts=bz - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: recognizing memory sticks in tifm
Not really. Current svn has read-only support for legacy MS (not mspro yet). I'm still working on it. The major difference: mmc/sd has an open spec, sony ms has none. --- Norbert Preining <[EMAIL PROTECTED]> wrote: > Hi Alex, hi all, > > I have an Acer TM3012 with > 0a:09.2 Mass storage controller: Texas Instruments 5-in-1 Multimedia Card > Reader (SD/MMC/MS/MS > PRO/xD) > > but only when I plug in an SD card I get a device created, while > plugging in a Memory Stick I do not get any reaction of the kernel. > > This is with kernel 2.6.22-rc6. > > Is this feature missing or could it be a configuration error (.config if > you need it). > > Inserting a sd card gives: > tifm_core: MMC/SD card detected in socket 0:1 > mmcblk0: mmc0:bffc SD02G 1985024KiB > mmcblk0: p1 > > From the "MMC/SD card detected" I assumed that MS should work, too. > > Thanks for any clarification. > > Best wishes > > Norbert > > --- > Dr. Norbert Preining <[EMAIL PROTECTED]>Vienna University of > Technology > Debian Developer <[EMAIL PROTECTED]> Debian TeX Group > gpg DSA: 0x09C5B094 fp: 14DF 2E6C 0307 BE6D AD76 A9C0 D2BF 4AA3 09C5 > B094 > --- > MAENTWROG (n. Welsh) > The height by which the top of a wave exceeds the heigh to which you > have rolled up your trousers. > --- Douglas Adams, The Meaning of Liff > Never miss an email again! Yahoo! Toolbar alerts you the instant new Mail arrives. http://tools.search.yahoo.com/toolbar/features/mail/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: recognizing memory sticks in tifm
Not really. Current svn has read-only support for legacy MS (not mspro yet). I'm still working on it. The major difference: mmc/sd has an open spec, sony ms has none. --- Norbert Preining [EMAIL PROTECTED] wrote: Hi Alex, hi all, I have an Acer TM3012 with 0a:09.2 Mass storage controller: Texas Instruments 5-in-1 Multimedia Card Reader (SD/MMC/MS/MS PRO/xD) but only when I plug in an SD card I get a device created, while plugging in a Memory Stick I do not get any reaction of the kernel. This is with kernel 2.6.22-rc6. Is this feature missing or could it be a configuration error (.config if you need it). Inserting a sd card gives: tifm_core: MMC/SD card detected in socket 0:1 mmcblk0: mmc0:bffc SD02G 1985024KiB mmcblk0: p1 From the MMC/SD card detected I assumed that MS should work, too. Thanks for any clarification. Best wishes Norbert --- Dr. Norbert Preining [EMAIL PROTECTED]Vienna University of Technology Debian Developer [EMAIL PROTECTED] Debian TeX Group gpg DSA: 0x09C5B094 fp: 14DF 2E6C 0307 BE6D AD76 A9C0 D2BF 4AA3 09C5 B094 --- MAENTWROG (n. Welsh) The height by which the top of a wave exceeds the heigh to which you have rolled up your trousers. --- Douglas Adams, The Meaning of Liff Never miss an email again! Yahoo! Toolbar alerts you the instant new Mail arrives. http://tools.search.yahoo.com/toolbar/features/mail/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [tifm] Infinite loop
--- Renato S. Yamane [EMAIL PROTECTED] wrote: When I insert a SDCard in my laptop M45-S355 my system crash because tifm start a infinite loop. See below more detail about SD/MMC Card and infinite loop. If you are using the built-in version found in 2.6.21 then it is a known problem. You can use the driver from here, to fix this: https://developer.berlios.de/project/showfiles.php?group_id=5510 (or wait for 2.6.22) Luggage? GPS? Comic books? Check out fitting gifts for grads at Yahoo! Search http://search.yahoo.com/search?fr=oni_on_mailp=graduation+giftscs=bz - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Freezeable workqueues [Was: 2.6.22-rc1: Broken suspend on SMP with tifm]
> > > > - Do we need freezeable workqueues ? > > > > Well, we have at least one case in which they appear to be useful. > I need freezeable wq exactly for the fact that they are synchronized with suspend/resume. My workitem may do device_register/unregister and it can (and will be) scheduled from irq handler during resume. As far as I understand, before freezeable wqs, kthreads were the only way to achieve this behavior, which is less convenient. Need Mail bonding? Go to the Yahoo! Mail Q for great tips from Yahoo! Answers users. http://answers.yahoo.com/dir/?link=list=396546091 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Freezeable workqueues [Was: 2.6.22-rc1: Broken suspend on SMP with tifm]
- Do we need freezeable workqueues ? Well, we have at least one case in which they appear to be useful. I need freezeable wq exactly for the fact that they are synchronized with suspend/resume. My workitem may do device_register/unregister and it can (and will be) scheduled from irq handler during resume. As far as I understand, before freezeable wqs, kthreads were the only way to achieve this behavior, which is less convenient. Need Mail bonding? Go to the Yahoo! Mail QA for great tips from Yahoo! Answers users. http://answers.yahoo.com/dir/?link=listsid=396546091 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.22-rc1: Broken suspend on SMP with tifm
I don't have any particular need in multithreaded wq, but I do need it freezeable. Freezeability simplifies things quite a lot. This is ok with me: > -#define create_freezeable_workqueue(name) __create_workqueue((name), 0, 1) > +#define create_freezeable_workqueue(name) __create_workqueue((name), 1, 1) 8:00? 8:25? 8:40? Find a flick in no time with the Yahoo! Search movie showtime shortcut. http://tools.search.yahoo.com/shortcuts/#news - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.22-rc1: Broken suspend on SMP with tifm
I don't have any particular need in multithreaded wq, but I do need it freezeable. Freezeability simplifies things quite a lot. This is ok with me: -#define create_freezeable_workqueue(name) __create_workqueue((name), 0, 1) +#define create_freezeable_workqueue(name) __create_workqueue((name), 1, 1) 8:00? 8:25? 8:40? Find a flick in no time with the Yahoo! Search movie showtime shortcut. http://tools.search.yahoo.com/shortcuts/#news - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
--- Pierre Ossman <[EMAIL PROTECTED]> wrote: > Sergey Yanovich wrote: > > I have compiled v2.6.21 with git-mmc.patch of v2.6.21-rc7.mm2. > > After [tifm_sd] is loaded. My smoke test script at > > > > http://bugzilla.kernel.org/attachment.cgi?id=11240=view > > > > reliably hangs suspend. > > I really wish you would stop removing me from cc... > > Suspend is broken in -mm for all controllers. There is a bit of a race between > the resume and remove functions that causes things to deadlock. I have a fix, > but I'm working on pushing a lot of stuff to Linus right now so it might take > a > few days before things hit the repos. > I prepared a tarball with 2.6.21 compatible driver (tifm-0.8e on berlios). I don't see any problems on stock 2.6.21.1. You may want to test it on your machine. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
> After [tifm_sd] is loaded. My smoke test script at > > http://bugzilla.kernel.org/attachment.cgi?id=11240=view > > reliably hangs suspend. > What is "modprobe tifm"? What modules have you loaded when it hangs? __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
After [tifm_sd] is loaded. My smoke test script at http://bugzilla.kernel.org/attachment.cgi?id=11240action=view reliably hangs suspend. What is modprobe tifm? What modules have you loaded when it hangs? __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
--- Pierre Ossman [EMAIL PROTECTED] wrote: Sergey Yanovich wrote: I have compiled v2.6.21 with git-mmc.patch of v2.6.21-rc7.mm2. After [tifm_sd] is loaded. My smoke test script at http://bugzilla.kernel.org/attachment.cgi?id=11240action=view reliably hangs suspend. I really wish you would stop removing me from cc... Suspend is broken in -mm for all controllers. There is a bit of a race between the resume and remove functions that causes things to deadlock. I have a fix, but I'm working on pushing a lot of stuff to Linus right now so it might take a few days before things hit the repos. I prepared a tarball with 2.6.21 compatible driver (tifm-0.8e on berlios). I don't see any problems on stock 2.6.21.1. You may want to test it on your machine. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
> It seems a bit out-of-date. Here is the output: > It clearly says there that the driver is for 2.6.20. The changes needed for 2.6.21 are actually very small (couple of fields in the mmc layer were renamed). In general, it is impossible to maintain out-of-tree driver in sync with kernel tree - too many changes are committed into it. The -mm version obviously fits its tree. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
> > I have submitted my version only after I have failed to find a stable > version of your driver for the current kernel. If there is one, just > post link to the patch. If not, let's make one. > As I already said, I'm not aware of any issues with version 0.8 of the driver. If you have any problems with it, I'll be glad to fix them. The version in question was recently committed into the -mm tree. Otherwise, it's available from the project's website for a couple of months now: http://developer.berlios.de/project/showfiles.php?group_id=5510 __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
I have submitted my version only after I have failed to find a stable version of your driver for the current kernel. If there is one, just post link to the patch. If not, let's make one. As I already said, I'm not aware of any issues with version 0.8 of the driver. If you have any problems with it, I'll be glad to fix them. The version in question was recently committed into the -mm tree. Otherwise, it's available from the project's website for a couple of months now: http://developer.berlios.de/project/showfiles.php?group_id=5510 __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
It seems a bit out-of-date. Here is the output: It clearly says there that the driver is for 2.6.20. The changes needed for 2.6.21 are actually very small (couple of fields in the mmc layer were renamed). In general, it is impossible to maintain out-of-tree driver in sync with kernel tree - too many changes are committed into it. The -mm version obviously fits its tree. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
--- Pierre Ossman <[EMAIL PROTECTED]> wrote: > Sergey Yanovich wrote: > > > > I have found it easier to rewrite the driver, than to fix. > > Before you get your hopes up, this development model is not one that will get > your code merged upstream. You should really try to work with Alex, not side > step him. Drivers are rarely complex enough to warrant, or even have room > for, a > rewrite. And judging from your code it looks more like reorganising the code > that's already there. It is a sad truth. Instead of raising real issues that may remain in the driver, I was presented with "non-proof" that bus-adapter-device architecture I'm using is somehow bad and the driver should be turned into a monolithic blob, using config variables to disable unneeded functionality. Considering, that udev handles automatic loading of the drivers just fine (so it's not an end user issue at any rate), I don't see any justification for the change. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
--- Pierre Ossman [EMAIL PROTECTED] wrote: Sergey Yanovich wrote: I have found it easier to rewrite the driver, than to fix. Before you get your hopes up, this development model is not one that will get your code merged upstream. You should really try to work with Alex, not side step him. Drivers are rarely complex enough to warrant, or even have room for, a rewrite. And judging from your code it looks more like reorganising the code that's already there. It is a sad truth. Instead of raising real issues that may remain in the driver, I was presented with non-proof that bus-adapter-device architecture I'm using is somehow bad and the driver should be turned into a monolithic blob, using config variables to disable unneeded functionality. Considering, that udev handles automatic loading of the drivers just fine (so it's not an end user issue at any rate), I don't see any justification for the change. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
> > I am not in any way argue that your driver architecture is wrong or that you > should change anything. My point was simple. [tifm_sd] can only work with > [tifm_7xx1]. If you add support for let's say [tifm_8xx2] in the future, which > would have port offsets different that [tifm_7xx1], you would also need a > completely new modules for slots (sd, ms, etc). > Does not this constitutes an unbounded speculation? And then, what would you propose to do with adapters that have SD support disabled? There are quite a few of those in the wild, as of right now (SD support is provided by bundled SDHCI on such systems, if at all). Similar argument goes for other media types as well - many controllers have xD support disabled too (I think you have one of those - Sony really values its customers). After all, it is not healthy to have dead code in the kernel. On the other hand, if TI puts out a controller which is functionally identical, but has different register map, it wouldn't be hard to refactor the code. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
--- Sergey Yanovich <[EMAIL PROTECTED]> wrote: > > > For a typical, non-linux-geek user there are just two states of the > > > device - > > > available and not available. Ububtu is famous for its end-user support. > > > They ship your driver since linux-2.6.17. But they pack it in one module. > > > And that is _much_ easier, then a hotplug script. > > > > No, we ship a udev script. > > OK, me bad for using the present tense. But you had a single module > in Oct 2006, didn't you? And maybe, you would like to post the script. Here's a simple udev rule that will do what you want: SUBSYSTEM=="tifm", ACTION=="add", TIFM_CARD_TYPE=="SD", RUN+="/sbin/modprobe tifm_sd" (just stick it somewhere in the udev rules). You shall take into consideration that mmc currently lacks uevent support, so mmc_block must be loaded manually (this is a long pending todo thingy; supposedly it waits for the first sdio driver). There's no direct dependency of tifm_sd on mmc_block (only on mmc_core). May be I'll add a modalias entry later so the explicit rule will not be needed. As a side note: I have some very good reasons for the current driver architecture. You may want to look them up in the mail archive (I outlined them during the initial driver submission). __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
--- Sergey Yanovich [EMAIL PROTECTED] wrote: For a typical, non-linux-geek user there are just two states of the device - available and not available. Ububtu is famous for its end-user support. They ship your driver since linux-2.6.17. But they pack it in one module. And that is _much_ easier, then a hotplug script. No, we ship a udev script. OK, me bad for using the present tense. But you had a single module in Oct 2006, didn't you? And maybe, you would like to post the script. Here's a simple udev rule that will do what you want: SUBSYSTEM==tifm, ACTION==add, TIFM_CARD_TYPE==SD, RUN+=/sbin/modprobe tifm_sd (just stick it somewhere in the udev rules). You shall take into consideration that mmc currently lacks uevent support, so mmc_block must be loaded manually (this is a long pending todo thingy; supposedly it waits for the first sdio driver). There's no direct dependency of tifm_sd on mmc_block (only on mmc_core). May be I'll add a modalias entry later so the explicit rule will not be needed. As a side note: I have some very good reasons for the current driver architecture. You may want to look them up in the mail archive (I outlined them during the initial driver submission). __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
I am not in any way argue that your driver architecture is wrong or that you should change anything. My point was simple. [tifm_sd] can only work with [tifm_7xx1]. If you add support for let's say [tifm_8xx2] in the future, which would have port offsets different that [tifm_7xx1], you would also need a completely new modules for slots (sd, ms, etc). Does not this constitutes an unbounded speculation? And then, what would you propose to do with adapters that have SD support disabled? There are quite a few of those in the wild, as of right now (SD support is provided by bundled SDHCI on such systems, if at all). Similar argument goes for other media types as well - many controllers have xD support disabled too (I think you have one of those - Sony really values its customers). After all, it is not healthy to have dead code in the kernel. On the other hand, if TI puts out a controller which is functionally identical, but has different register map, it wouldn't be hard to refactor the code. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
> Finally, tifm_sd module needs to be manually inserted. By the way, the driver emits custom uevent when the new card is detected. I was going to look at this some day in the future, but if you want to mess a little with hotplug scripts the issue can be easily solved. As I already said before, many of the complications exist because this is an universal adapter, and memorystick support is quite near in the queue. A good hotplug script will, therefore, look at the "TIFM_CARD_TYPE" event var and load the appropriate media driver. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
Finally, tifm_sd module needs to be manually inserted. By the way, the driver emits custom uevent when the new card is detected. I was going to look at this some day in the future, but if you want to mess a little with hotplug scripts the issue can be easily solved. As I already said before, many of the complications exist because this is an universal adapter, and memorystick support is quite near in the queue. A good hotplug script will, therefore, look at the TIFM_CARD_TYPE event var and load the appropriate media driver. __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
Have you looked at the last version (0.8)? It fixed all outstanding issues (as far as I know). --- Sergey Yanovich <[EMAIL PROTECTED]> wrote: - Hi, The device is present in many notebooks. Notebooks depend heavily onsuspend/resume functionality. tifm_core/7xx1/sd family is an ambitous,but uncompleted project. It used to crash on resuming, or hang up onsuspending. A less common failure used to be trigerred by a fast cardinsert/removal sequence. Finally, tifm_sd module needs to be manuallyinserted. I have found it easier to rewrite the driver, than to fix. This driveris kind of mutant. The bones are taken from sdhci and omap, the meat -from tifm_*. It contains all features (and bugs except named above) oftifm_* as it was in kernel 2.6.21-rc7. I have been testing this version since linux-2.6.18 (daily readingphotos from cards, daily suspending/resuming) without a single glitch. This patch only provides sources. [PATCH1/2] [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7 Kernel configuration in this message. [PATCH2/2] [mmc] alternative TIFM driver config for 2.6.21-rc7 Alex Dubov has done exceptionally great lots of work to teach linuxspeak to TIFM. This is just a reorganization of his project. The driver seems to be practically stable, but it definitely must betested by more people. Please also report any issues with this driverto linux bug#8352 so that valuable info is not lost. Best regards, Sergey Yanovich __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
Have you looked at the last version (0.8)? It fixed all outstanding issues (as far as I know). --- Sergey Yanovich [EMAIL PROTECTED] wrote: - Hi, The device is present in many notebooks. Notebooks depend heavily onsuspend/resume functionality. tifm_core/7xx1/sd family is an ambitous,but uncompleted project. It used to crash on resuming, or hang up onsuspending. A less common failure used to be trigerred by a fast cardinsert/removal sequence. Finally, tifm_sd module needs to be manuallyinserted. I have found it easier to rewrite the driver, than to fix. This driveris kind of mutant. The bones are taken from sdhci and omap, the meat -from tifm_*. It contains all features (and bugs except named above) oftifm_* as it was in kernel 2.6.21-rc7. I have been testing this version since linux-2.6.18 (daily readingphotos from cards, daily suspending/resuming) without a single glitch. This patch only provides sources. [PATCH1/2] [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7 Kernel configuration in this message. [PATCH2/2] [mmc] alternative TIFM driver config for 2.6.21-rc7 Alex Dubov has done exceptionally great lots of work to teach linuxspeak to TIFM. This is just a reorganization of his project. The driver seems to be practically stable, but it definitely must betested by more people. Please also report any issues with this driverto linux bug#8352 so that valuable info is not lost. Best regards, Sergey Yanovich __ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tifm_sd: add missing \n
In fact, this is fixed in already queued patch set. --- Daniel Drake <[EMAIL PROTECTED]> wrote: > Noticed this in user logs. > > Signed-off-by: Daniel Drake <[EMAIL PROTECTED]> > > Index: linux/drivers/mmc/tifm_sd.c > === > --- linux.orig/drivers/mmc/tifm_sd.c > +++ linux/drivers/mmc/tifm_sd.c > @@ -676,7 +676,7 @@ static void tifm_sd_abort(unsigned long > struct tifm_sd *host = (struct tifm_sd*)data; > > printk(KERN_ERR DRIVER_NAME > -": card failed to respond for a long period of time"); > +": card failed to respond for a long period of time\n"); > > tifm_sd_terminate(host); > tifm_eject(host->dev); > Food fight? Enjoy some healthy debate in the Yahoo! Answers Food & Drink Q http://answers.yahoo.com/dir/?link=list=396545367 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tifm_sd: add missing \n
In fact, this is fixed in already queued patch set. --- Daniel Drake [EMAIL PROTECTED] wrote: Noticed this in user logs. Signed-off-by: Daniel Drake [EMAIL PROTECTED] Index: linux/drivers/mmc/tifm_sd.c === --- linux.orig/drivers/mmc/tifm_sd.c +++ linux/drivers/mmc/tifm_sd.c @@ -676,7 +676,7 @@ static void tifm_sd_abort(unsigned long struct tifm_sd *host = (struct tifm_sd*)data; printk(KERN_ERR DRIVER_NAME -: card failed to respond for a long period of time); +: card failed to respond for a long period of time\n); tifm_sd_terminate(host); tifm_eject(host-dev); Food fight? Enjoy some healthy debate in the Yahoo! Answers Food Drink QA. http://answers.yahoo.com/dir/?link=listsid=396545367 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Weird MMC errors: 2 of 2 - inconsistent state after data crc error
Problem 2: After a data crc error all subsequent commands fail. May it be caused by stop command leaving card in some bad state (something clearable by SEND_STATUS)? On the other hand, is there a real need to issue a stop command in case main command failed? Example: Mar 14 09:25:13 Tirion kernel: mmcblk0:<7>mmc0: starting CMD18 arg flags 0035 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: setting dma for 8 blocks Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x12, arg: 0x0, mask: 0xb100 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 40, flags 1 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0xc, arg: 0x0, mask: 0x2900 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 9 Mar 14 09:25:13 Tirion kernel: mmc0: req done (CMD18): 0/2/1: 0900 Mar 14 09:25:13 Tirion kernel: mmcblk0: error 2 transferring data Mar 14 09:25:13 Tirion kernel: end_request: I/O error, dev mmcblk0, sector 0 Mar 14 09:25:13 Tirion kernel: printk: 342 messages suppressed. Mar 14 09:25:13 Tirion kernel: Buffer I/O error on device mmcblk0, logical block 0 Mar 14 09:25:13 Tirion kernel: mmc0: starting CMD18 arg flags 0035 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: setting dma for 8 blocks Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x12, arg: 0x0, mask: 0xb100 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 0 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0xc, arg: 0x0, mask: 0x2900 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 8 Mar 14 09:25:13 Tirion kernel: mmc0: req done (CMD18): 1/0/1: Sucker-punch spam with award-winning protection. Try the free Yahoo! Mail Beta. http://advision.webevents.yahoo.com/mailbeta/features_spam.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Weird MMC errors: 1 of 2 - bad ocr value
Recently, I've obtained a bug report concerning an MMC card. Two problems are described, both sporadic. Problem 1: illegal ocr value is returned. You may notice, in the non-working case, obviously incorrect ocr value (0x) is returned. The card won't work after this, unless reinserted. What, to your opinion, shall we do about it? Normal case - everything works: -- Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1001, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 00ff8000 Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1001, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 00ff8000 Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 80ff8000 Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD2 arg flags 0067 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x2, arg: 0x0, mask: 0x1240 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD2): 0/0/0: 06000131 364d2020 20011d43 332479ad --- --- Error case: OCR value is bad, card is left in bad state and won't work: - Mar 14 09:15:24 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1001, flags 0 Mar 14 09:15:24 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 00ff8000 Mar 14 09:15:24 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:15:24 Tirion kernel: mmc0: req done (CMD1): 0/0/0: Mar 14 09:15:24 Tirion kernel: mmc0: starting CMD2 arg flags 0067 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x2, arg: 0x0, mask: 0x1240 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 0 Mar 14 09:15:24 Tirion kernel: mmc0: req done (CMD2): 1/0/0: - Don't get soaked. Take a quick peek at the forecast with the Yahoo! Search weather shortcut. http://tools.search.yahoo.com/shortcuts/#loc_weather - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Weird MMC errors: 1 of 2 - bad ocr value
Recently, I've obtained a bug report concerning an MMC card. Two problems are described, both sporadic. Problem 1: illegal ocr value is returned. You may notice, in the non-working case, obviously incorrect ocr value (0x) is returned. The card won't work after this, unless reinserted. What, to your opinion, shall we do about it? Normal case - everything works: -- Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1001, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 00ff8000 Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1001, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 00ff8000 Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 80ff8000 Mar 14 09:15:39 Tirion kernel: mmc0: starting CMD2 arg flags 0067 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x2, arg: 0x0, mask: 0x1240 Mar 14 09:15:39 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:15:39 Tirion kernel: mmc0: req done (CMD2): 0/0/0: 06000131 364d2020 20011d43 332479ad --- --- Error case: OCR value is bad, card is left in bad state and won't work: - Mar 14 09:15:24 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1001, flags 0 Mar 14 09:15:24 Tirion kernel: mmc0: req done (CMD1): 0/0/0: 00ff8000 Mar 14 09:15:24 Tirion kernel: mmc0: starting CMD1 arg 0030 flags 0061 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x1, arg: 0x30, mask: 0x1340 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:15:24 Tirion kernel: mmc0: req done (CMD1): 0/0/0: Mar 14 09:15:24 Tirion kernel: mmc0: starting CMD2 arg flags 0067 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x2, arg: 0x0, mask: 0x1240 Mar 14 09:15:24 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 0 Mar 14 09:15:24 Tirion kernel: mmc0: req done (CMD2): 1/0/0: - Don't get soaked. Take a quick peek at the forecast with the Yahoo! Search weather shortcut. http://tools.search.yahoo.com/shortcuts/#loc_weather - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Weird MMC errors: 2 of 2 - inconsistent state after data crc error
Problem 2: After a data crc error all subsequent commands fail. May it be caused by stop command leaving card in some bad state (something clearable by SEND_STATUS)? On the other hand, is there a real need to issue a stop command in case main command failed? Example: Mar 14 09:25:13 Tirion kernel: mmcblk0:7mmc0: starting CMD18 arg flags 0035 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: setting dma for 8 blocks Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x12, arg: 0x0, mask: 0xb100 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 1, flags 0 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 40, flags 1 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0xc, arg: 0x0, mask: 0x2900 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 9 Mar 14 09:25:13 Tirion kernel: mmc0: req done (CMD18): 0/2/1: 0900 Mar 14 09:25:13 Tirion kernel: mmcblk0: error 2 transferring data Mar 14 09:25:13 Tirion kernel: end_request: I/O error, dev mmcblk0, sector 0 Mar 14 09:25:13 Tirion kernel: printk: 342 messages suppressed. Mar 14 09:25:13 Tirion kernel: Buffer I/O error on device mmcblk0, logical block 0 Mar 14 09:25:13 Tirion kernel: mmc0: starting CMD18 arg flags 0035 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: setting dma for 8 blocks Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0x12, arg: 0x0, mask: 0xb100 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 0 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: executing opcode 0xc, arg: 0x0, mask: 0x2900 Mar 14 09:25:13 Tirion kernel: tifm_sd tifm_sd0:1: host event: host_status 80, flags 8 Mar 14 09:25:13 Tirion kernel: mmc0: req done (CMD18): 1/0/1: Sucker-punch spam with award-winning protection. Try the free Yahoo! Mail Beta. http://advision.webevents.yahoo.com/mailbeta/features_spam.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Recent and not-so problems with tifm_sd driver
> > > > mmc_rescan > > mmc_register_card > > device_add > > mmc_block_probe > > mmc_block_alloc > > -> queue thread starts running > > add_disk > > -> issues a lot of requests; card fails, my drivers calls > > mmc_remove_host, which in correction: my driver schedules (wakes kthread in 0.7) mmc_remove_host - noticed it only now > > turn calls device_del, though we are still in device_add > > > That's why I think that simply flushing the workqueue is enough. If workqueue is empty we know for sure that device_add has exited. Expecting? Get great news right away with email Auto-Check. Try the Yahoo! Mail Beta. http://advision.webevents.yahoo.com/mailbeta/newmail_tools.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Recent and not-so problems with tifm_sd driver
--- Pierre Ossman <[EMAIL PROTECTED]> wrote: > Alex Dubov wrote: > > > > You'll agree, I think, that add_disk in mmc_block_probe issues a lot of > > requests (reads > partition > > table, fs superblocks and such - plenty of room for critical errors). Then, > > driver's remove > method > > will not be called before driver's probe method had finished. So mmc_block > > is quite involved, > even > > though it does not affect the problem's resolution. > > I agree that mmc_block's probe method will generate a whole bunch of requests. > But I don't see how that can be called given the scenario you describe. mmc_rescan mmc_register_card device_add mmc_block_probe mmc_block_alloc -> queue thread starts running add_disk -> issues a lot of requests; card fails, my drivers calls mmc_remove_host, which in turn calls device_del, though we are still in device_add Have a burning question? Go to www.Answers.yahoo.com and get answers from real people who know. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Recent and not-so problems with tifm_sd driver
--- Pierre Ossman [EMAIL PROTECTED] wrote: Alex Dubov wrote: You'll agree, I think, that add_disk in mmc_block_probe issues a lot of requests (reads partition table, fs superblocks and such - plenty of room for critical errors). Then, driver's remove method will not be called before driver's probe method had finished. So mmc_block is quite involved, even though it does not affect the problem's resolution. I agree that mmc_block's probe method will generate a whole bunch of requests. But I don't see how that can be called given the scenario you describe. mmc_rescan mmc_register_card device_add mmc_block_probe mmc_block_alloc - queue thread starts running add_disk - issues a lot of requests; card fails, my drivers calls mmc_remove_host, which in turn calls device_del, though we are still in device_add Have a burning question? Go to www.Answers.yahoo.com and get answers from real people who know. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Recent and not-so problems with tifm_sd driver
mmc_rescan mmc_register_card device_add mmc_block_probe mmc_block_alloc - queue thread starts running add_disk - issues a lot of requests; card fails, my drivers calls mmc_remove_host, which in correction: my driver schedules (wakes kthread in 0.7) mmc_remove_host - noticed it only now turn calls device_del, though we are still in device_add That's why I think that simply flushing the workqueue is enough. If workqueue is empty we know for sure that device_add has exited. Expecting? Get great news right away with email Auto-Check. Try the Yahoo! Mail Beta. http://advision.webevents.yahoo.com/mailbeta/newmail_tools.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Recent and not-so problems with tifm_sd driver
> > This is hard to trigger problem, so I'll spare you the rather lengthy log. > > It happens if card timeouts and mmc_remove_host is called while > > mmc_register_card is still in > > progress (the hint was in crash dump). If I sleep before remove, it gives > > the > mmc_register_card > > chance to finish/mark card as dead and everything's fine. > > The remove callback of mmc_block is apparently not executed in this case > > (probably because > device > > has not finished registering). > > Let's see, you manage to call mmc_remove_host() when a detection is pending in > the workqueue? That could indeed generate new request (up until > mmc_free_host() > is called), but as the card is removed the mmc layer shouldn't be able to > detect > anything, hence mmc_block should never get involved. > You'll agree, I think, that add_disk in mmc_block_probe issues a lot of requests (reads partition table, fs superblocks and such - plenty of room for critical errors). Then, driver's remove method will not be called before driver's probe method had finished. So mmc_block is quite involved, even though it does not affect the problem's resolution. Besides this, I have a new version of the tifm driver to submit. It fixes quite a few problems and improves performance a little. Hopefully, it's better tested than the previous one. Finding fabulous fares is fun. Let Yahoo! FareChase search your favorite travel sites to find flight and hotel bargains. http://farechase.yahoo.com/promo-generic-14795097 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Recent and not-so problems with tifm_sd driver
> > I don't see how that is possible. mmc_block's remove routine waits for mmcqd > to > exit, so there can't be any code still alive that has a request going. (I am > also completely unable to reproduce this problem here). > > Add more printk:s do verify how the code in mmc_block executes. > This is hard to trigger problem, so I'll spare you the rather lengthy log. It happens if card timeouts and mmc_remove_host is called while mmc_register_card is still in progress (the hint was in crash dump). If I sleep before remove, it gives the mmc_register_card chance to finish/mark card as dead and everything's fine. The remove callback of mmc_block is apparently not executed in this case (probably because device has not finished registering). My proposition: lets flush the workqueue first thing in the mmc_remove_host (and make sure that nothing gets scheduled into it after this). Don't pick lemons. See all the new 2007 cars at Yahoo! Autos. http://autos.yahoo.com/new_cars.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] incorrect behavior on resume
> I don't see that - as I say above, the correct sequence is: > > - host device resume > - calls mmc_resume_host() > - child's device resume (mmc_blk_resume) > - mmc_queue_resume() > Of course, I understand that this is a correct sequence. It simply was not obvious to me that host will issue requests even if mmc_resume_host is not called. I'm trying to implement a hotplugable host, so I assumed that if host was removed during the suspended state there's no need to resume it. Food fight? Enjoy some healthy debate in the Yahoo! Answers Food & Drink Q http://answers.yahoo.com/dir/?link=list=396545367 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mmc] incorrect behavior on resume
I don't see that - as I say above, the correct sequence is: - host device resume - calls mmc_resume_host() - child's device resume (mmc_blk_resume) - mmc_queue_resume() Of course, I understand that this is a correct sequence. It simply was not obvious to me that host will issue requests even if mmc_resume_host is not called. I'm trying to implement a hotplugable host, so I assumed that if host was removed during the suspended state there's no need to resume it. Food fight? Enjoy some healthy debate in the Yahoo! Answers Food Drink QA. http://answers.yahoo.com/dir/?link=listsid=396545367 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/