Re: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
On Tue, Oct 21, 2014 at 03:18:58AM +, Thomas Shao wrote: > > In some situation, the user is not able to enable guest VM to sync with > external > time source, like NTP. But the host is still synced with a trusted time > source. But the guest *is* networked, right? (Otherwise syncing the guest's clock is pointless.) > I've got some feedbacks from Richard and Mike, including reference NTP > implementation > and do the adjustment in the host side. I've already referenced some NTP > design in > my patch. I would consider my patch as a simplified implementation. I really don't think we want a half baked servo in some random driver. Instead, why not present the time difference using a standard interface? > I've also considered > the host side implementation. But in host, we can only set time but not > gradually slew/adjust > time, Why not implement adjustment in the host? > which is not acceptable for the time sync solution.We still recommend user to > configure > NTP on the guest, which provides better accuracy. But if NTP is not > applicable, this could be > another option. You did not really answer any of my objections, nor did you consider the alternative ideas which I offered. Would you care to address those? Thanks, Richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
On Tue, 21 Oct 2014, Thomas Shao wrote: > > I still do not have a consistent argument from you WHY you need to abuse > > do_adjtimex() to do that host - guest synchronization in the first place. > > > > I need a function to gradually slew guest time. do_adjtimex() provides all > the > functionality. Also I could not find any other exposed func to do this. I'd > like to > hear any feedback from you for this. As Richard and others told you already, there are various options: 1) Use NTP on that private network, which does not involve any kernel changes at all. Your argument, that this is hard for IT-Admins to set up is just ridiculous. If an IT-Admin is not able to set that up, then he should better stay away from setting up a guest in the first place, really. 2) As pointed out already by others PPS/PTP might be a proper solution for this. All it takes is a pair of timestamps (host/guest) injected into the proper subsystem and a controlling daemon on the guest side. That would also avoid the problem of running NTPd and your kernel side poor mans NTPd at the same time. That pseudo NTP thing is just hilarious, really. You take the host time stamp in timesync_onchannelcallback() and schedule work. From the work queue you correlate the host time stamp to the current time of the guest. So you correlate time stamps which can be an arbitrary time apart. Brilliant solution that, really. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
On Mon, Oct 20, 2014 at 11:02:13PM -0500, Jeff Epler wrote: > It's interesting to imagine that a virtualization host could present a > time service to the guest *userspace*, even when the guest is not > otherwise exposed to the internet at large. This could take the form of > an NTP server on a private network, or as an implementation of a time > source directly usable by ntpd in the guest, for instance as an emulated > serial port with synthetic NEMA GPS signal + PPS signal, for instance. If the idea is to avoid bothering the guest user space, in order to be convenient, then the host can provide a synthetic PPS, to be used by the kernel's hardpps logic. Thanks, Richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
On Tue, 21 Oct 2014, Thomas Shao wrote: > I didn't find a way to detect whether NTPd is running in the hyper-v module. And you better do not try at all. > In http://doc.ntp.org/4.1.0/ntpd.htm, it mentioned: Normally, the > time is slewed if the offset is less than the step threshold, which > is 128 ms by default, and stepped if above the threshold. > > In my implementation, I use 100ms as the threshold (maybe I should > change to 128?). If the time difference is less than 100ms, I just > do nothing. So, if NTPd is running, ideally it could keep the time > drift less than 128, so the adjustment in my patch will not get > triggered. Your implementation has nothing to do with NTP at all. It's not even close to NTP. It's a random hack to inject host time or slew into timekeeping with the precision of a random number generator. > And moreover, by default, the guest-host time sync is turn > off. There is a module parameter to control it. We'll also document > customer that do not turn on this if NTP is configured. Pretty well thought out mechanism to ensure that people will get it wrong in the first place. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
On Tue, 21 Oct 2014, Thomas Shao wrote: > I'm also thinking if NTPd could expose some interface to allow other > application to directly provide time source for it to consume. In my > opinion, emulating the ntp source should be very hard and error > prone. Well, if done right it would be pretty precise. At least way better than the random number you feed into do_adjtimex(). Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: fix memory leak / bad pointer freeing for chanlist
On 20/10/14 22:38, Hartley Sweeten wrote:
On Monday, October 20, 2014 7:11 AM, Ian Abbott wrote:
As a follow-up to commit 6cab7a37f5c04 ("staging: comedi: (regression)
channel list must be set for COMEDI_CMD ioctl"), Hartley Sweeten pointed
out another couple of bugs stemming from commit 6cab7a37f5c04 ("staging:
comedi: comedi_fops: introduce __comedi_get_user_chanlist()").
Firstly, `do_cmdtest_ioctl()` never frees the kernel copy of the user
chanlist allocated by `__comedi_get_user_chanlist()`, so that memory is
leaked. Fix it by freeing the allocated kernel memory pointed to by
`cmd.chanlist` before that pointer is overwritten with its original
pointer to user memory before `cmd` is copied back to user-space.
Secondly, if `__comedi_get_user_chanlist()` returns an error,
`cmd->chanlist` is left unchanged and in fact will be a pointer to user
memory. This causes `do_cmd_ioctl()` to `goto cleanup` and call
`do_become_nonbusy()` which would attempt to free the memory pointed to
by the user-space pointer. Fix it by setting `cmd->chanlist` to NULL at
the start of `__comedi_get_user_chanlist()`.
Fixes: c6cd0eefb27b ("staging: comedi: comedi_fops: introduce
__comedi_get_user_chanlist()")
Reported-by: H Hartley Sweeten
Cc: # 3.15.y 3.16.y 3.17.y: 6cab7a37f5c04
Cc: # 3.15.y 3.16.y 3.17.y
---
Greg, this patch applies to your "staging-linus" branch.
---
drivers/staging/comedi/comedi_fops.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index a9b7fe5..a1788e8 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1462,6 +1462,7 @@ static int __comedi_get_user_chanlist(struct
comedi_device *dev,
unsigned int *chanlist;
int ret;
+ cmd->chanlist = NULL;
chanlist = memdup_user(user_chanlist,
cmd->chanlist_len * sizeof(unsigned int));
if (IS_ERR(chanlist))
I don't think this covers all the problems in your second issue above.
It does fix the attempt to free the memory pointed to by the user-space pointer
if the memdub_user() fails.
But in do_cmd_ioctl() we still have an issue if the s->do_cmdtest() fails or
the CMDF_BOGUS flag is set. There we restore the users cmd.chanlist
pointer to allow copy_to_user() to return the cmd/ But the kernel allocated
chanlist is not freed. The goto cleanup will again try to free the user-space
pointer.
No, 'async->cmd' is copied (via struct assignment) to local variable
'cmd' and it is the local 'cmd.chanlist' pointer that is set to the
original user-space pointer and the local 'cmd' that is copied back to
user-space. 'async->cmd.chanlist' still points to the kernel copy of
the chanlist that gets cleaned up by 'goto cleanup' / 'do_become_nonbusy().
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 RESEND] Tools: hv: vssdaemon: ignore the EBUSY on multiple freezing the same partition
If a partition appears mounted more than once in /proc/mounts, vss_do_freeze()
succeeds only for the first time and gets EBUSY (on freeze) or EINVAL (on
thaw) for the second time. The patch ignores these to make the backup feature
work.
Also improved the error handling in case a freeze operation fails.
Signed-off-by: Dexuan Cui
Reviewed-by: K. Y. Srinivasan
---
v2: Add "errno = 0;" before the ioctl()
(Unnecessary and removed now since we remove syslog() in vss_do_freeze() in v3)
v3: Remove the unsafe syslog() in vss_do_freeze(): that could write the disk.
Thaw the filesystems in case the freezing operation fails.
In main(), add syslog() when we check the return value of vss_operate().
[Oct 21, 2014] This is just a RESEND since the previous one I sent about a month
ago seems neglected. :-(
tools/hv/hv_vss_daemon.c | 48
1 file changed, 40 insertions(+), 8 deletions(-)
diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 6a213b8..1db9430 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -44,21 +44,39 @@ static struct sockaddr_nl addr;
#endif
-static int vss_do_freeze(char *dir, unsigned int cmd, char *fs_op)
+/* Don't use syslog() in the function since that can cause write to disk */
+static int vss_do_freeze(char *dir, unsigned int cmd)
{
int ret, fd = open(dir, O_RDONLY);
if (fd < 0)
return 1;
+
ret = ioctl(fd, cmd, 0);
- syslog(LOG_INFO, "VSS: %s of %s: %s\n", fs_op, dir, strerror(errno));
+
+ /*
+* If a partition is mounted more than once, only the first
+* FREEZE/THAW can succeed and the later ones will get
+* EBUSY/EINVAL respectively: there could be 2 cases:
+* 1) a user may mount the same partition to differnt directories
+* by mistake or on purpose;
+* 2) The subvolume of btrfs appears to have the same partition
+* mounted more than once.
+*/
+ if (ret) {
+ if ((cmd == FIFREEZE && errno == EBUSY) ||
+ (cmd == FITHAW && errno == EINVAL)) {
+ close(fd);
+ return 0;
+ }
+ }
+
close(fd);
return !!ret;
}
static int vss_operate(int operation)
{
- char *fs_op;
char match[] = "/dev/";
FILE *mounts;
struct mntent *ent;
@@ -68,11 +86,9 @@ static int vss_operate(int operation)
switch (operation) {
case VSS_OP_FREEZE:
cmd = FIFREEZE;
- fs_op = "freeze";
break;
case VSS_OP_THAW:
cmd = FITHAW;
- fs_op = "thaw";
break;
default:
return -1;
@@ -93,15 +109,23 @@ static int vss_operate(int operation)
root_seen = 1;
continue;
}
- error |= vss_do_freeze(ent->mnt_dir, cmd, fs_op);
+ error |= vss_do_freeze(ent->mnt_dir, cmd);
+ if (error && operation == VSS_OP_FREEZE)
+ goto err;
}
endmntent(mounts);
if (root_seen) {
- error |= vss_do_freeze("/", cmd, fs_op);
+ error |= vss_do_freeze("/", cmd);
+ if (error && operation == VSS_OP_FREEZE)
+ goto err;
}
return error;
+err:
+ endmntent(mounts);
+ vss_operate(VSS_OP_THAW);
+ return error;
}
static int netlink_send(int fd, struct cn_msg *msg)
@@ -249,8 +273,16 @@ int main(void)
case VSS_OP_FREEZE:
case VSS_OP_THAW:
error = vss_operate(op);
- if (error)
+ syslog(LOG_INFO, "VSS: op=%s: %s\n",
+ op == VSS_OP_FREEZE ? "FREEZE" : "THAW",
+ error ? "failed" : "succeeded");
+
+ if (error) {
error = HV_E_FAIL;
+ syslog(LOG_ERR, "op=%d failed!", op);
+ syslog(LOG_ERR, "report it with these files:");
+ syslog(LOG_ERR, "/etc/fstab and /proc/mounts");
+ }
break;
default:
syslog(LOG_ERR, "Illegal op:%d\n", op);
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
> -Original Message- > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- > ow...@vger.kernel.org] On Behalf Of Thomas Gleixner > Sent: Tuesday, October 21, 2014 4:19 PM > To: Thomas Shao > Cc: gre...@linuxfoundation.org; LKML; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; KY Srinivasan; > John Stultz; Richard Cochran > Subject: RE: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for > do_adjtimex() > > On Tue, 21 Oct 2014, Thomas Shao wrote: > > > I still do not have a consistent argument from you WHY you need to > > > abuse > > > do_adjtimex() to do that host - guest synchronization in the first place. > > > > > > > I need a function to gradually slew guest time. do_adjtimex() provides > > all the functionality. Also I could not find any other exposed func to > > do this. I'd like to hear any feedback from you for this. > > As Richard and others told you already, there are various options: > > 1) Use NTP on that private network, which does not involve any kernel >changes at all. > >Your argument, that this is hard for IT-Admins to set up is just >ridiculous. If an IT-Admin is not able to set that up, then he >should better stay away from setting up a guest in the first place, >really. > > 2) As pointed out already by others PPS/PTP might be a proper solution >for this. > >All it takes is a pair of timestamps (host/guest) injected into the >proper subsystem and a controlling daemon on the guest side. That >would also avoid the problem of running NTPd and your kernel side >poor mans NTPd at the same time. > >That pseudo NTP thing is just hilarious, really. > >You take the host time stamp in timesync_onchannelcallback() and >schedule work. From the work queue you correlate the host time >stamp to the current time of the guest. So you correlate time >stamps which can be an arbitrary time apart. Brilliant solution >that, really. > OK. I'll investigate these options. Thanks. > Thanks, > > tglx > > -- > 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/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
> -Original Message- > From: Richard Cochran [mailto:richardcoch...@gmail.com] > Sent: Tuesday, October 21, 2014 4:14 PM > To: Thomas Shao > Cc: Thomas Gleixner; gre...@linuxfoundation.org; LKML; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; KY Srinivasan; John Stultz; Richard Cochran > Subject: Re: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for > do_adjtimex() > > On Tue, Oct 21, 2014 at 03:18:58AM +, Thomas Shao wrote: > > > > In some situation, the user is not able to enable guest VM to sync > > with external time source, like NTP. But the host is still synced with a > trusted time source. > > But the guest *is* networked, right? > > (Otherwise syncing the guest's clock is pointless.) > I believe it should be a valid scenario, that NTP is not available in the client, and we want the guest time sync with host. Other hypervisor like Xen, VMWare also provide the host-guest time sync feature. > > I've got some feedbacks from Richard and Mike, including reference NTP > > implementation and do the adjustment in the host side. I've already > > referenced some NTP design in my patch. I would consider my patch as a > simplified implementation. > > I really don't think we want a half baked servo in some random driver. > Instead, why not present the time difference using a standard interface? OK. I'll do more investigation. Could you let me know what's the standard interface for presenting time difference you mentioned here? Thanks! > > > I've also considered > > the host side implementation. But in host, we can only set time but > > not gradually slew/adjust time, > > Why not implement adjustment in the host? > > > which is not acceptable for the time sync solution.We still recommend > > user to configure NTP on the guest, which provides better accuracy. > > But if NTP is not applicable, this could be another option. > > You did not really answer any of my objections, nor did you consider the > alternative ideas which I offered. Would you care to address those? I do not agree the guest VM time adjustment should be handled by the host. In my opinion, the host should not involve in any Guest OS level operation. > > Thanks, > Richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: binder: move to the "real" part of the kernel
On Fri 2014-10-17 01:14:57, Greg Kroah-Hartman wrote: > On Thu, Oct 16, 2014 at 04:18:02PM +0200, Michael Kerrisk (man-pages) wrote: > > On Thu, Oct 16, 2014 at 2:47 PM, Greg Kroah-Hartman > > wrote: > > > From: Greg Kroah-Hartman > > > > > > The Android binder code has been "stable" for many years now. No matter > > > what comes in the future, we are going to have to support this API, so > > > might as well move it to the "real" part of the kernel as there's no > > > real work that needs to be done to the existing code. > > > > Where does one find the canonical documentation of the user-space API? > > There really is only one "canonical" thing, and that is in the libbinder > code in the Android userspace repository. And it's not really > "documentation" so much as, "a C file that interacts with the ioctls in > the binder kernel code" :( > > Think of this as just a random character driver with some funny ioctls > that will never get really documented as there is only one user of it. This is not random character driver, it is communication mechanism. It should _not_ be a character driver. And it should really be documented. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for do_adjtimex()
> -Original Message- > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- > ow...@vger.kernel.org] On Behalf Of Richard Cochran > Sent: Tuesday, October 21, 2014 4:21 PM > To: Jeff Epler > Cc: Thomas Shao; Thomas Gleixner; gre...@linuxfoundation.org; LKML; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > jasow...@redhat.com; KY Srinivasan; John Stultz; Richard Cochran > Subject: Re: [PATCH v2 1/2] timekeeping: add EXPORT_SYMBOL_GPL for > do_adjtimex() > > On Mon, Oct 20, 2014 at 11:02:13PM -0500, Jeff Epler wrote: > > It's interesting to imagine that a virtualization host could present a > > time service to the guest *userspace*, even when the guest is not > > otherwise exposed to the internet at large. This could take the form > > of an NTP server on a private network, or as an implementation of a > > time source directly usable by ntpd in the guest, for instance as an > > emulated serial port with synthetic NEMA GPS signal + PPS signal, for > instance. > > If the idea is to avoid bothering the guest user space, in order to be > convenient, then the host can provide a synthetic PPS, to be used by the > kernel's hardpps logic. > This is something I should consider. I'll do more investigation for this. Thanks Richard. > Thanks, > Richard > -- > 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/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: binder: move to the "real" part of the kernel
On Fri 2014-10-17 01:12:21, Greg Kroah-Hartman wrote:
> On Thu, Oct 16, 2014 at 10:09:04AM -0700, John Stultz wrote:
> > On Thu, Oct 16, 2014 at 5:47 AM, Greg Kroah-Hartman
> > wrote:
> > > From: Greg Kroah-Hartman
> > Are the Android guys comfortable with the ABI stability rules they'll
> > now face?
>
> Just because something is in staging, doesn't mean you don't have to
> follow the same ABI stability rules as the rest of the kernel. If a
> change had happened to this code that broke userspace in the past, I
> would have reverted it. So this should not be anything different from
> what has been happening inthe past.
Actually, there's big difference.
If Al Viro changes core filesystem in a way that breaks
staging/binder, binder is broken, and if it can't be fixed... well it
can't be fixed.
If Al Viro changes core filesystem in a way that breaks
drivers/binder, Al's change is going to be reverted.
It is really hard to review without API documentation. Normally, API
documentation is required for stuff like this.
For example: does it add new files in /proc?
Given that it is stable, can we get rid of binder_debug() and
especially BINDER_DEBUG_ENTRY stuff?
Checkpatch warns about 98 too long lines. Some of them could be fixed
easily.
This looks scary:
trace_binder_transaction_fd(t, fp->handle,
target_fd);
binder_debug(BINDER_DEBUG_TRANSACTION,
"fd %d -> %d\n",
fp->handle, target_fd);
/* TODO: fput? */
fp->handle = target_fd;
} break;
Could binder_transcation() be split to smaller functions according to
CodingStyle? 17 goto targets at the end of function are not exactly
easy to read.
ginder_thread_read/write also needs splitting.
binder_ioctl_write_read: just use direct return, no need to goto out
if it just returns.
proc->user_buffer_offset = vma->vm_start - (uintptr_t)proc->buffer;
mutex_unlock(&binder_mmap_lock);
#ifdef CONFIG_CPU_CACHE_VIPT
if (cache_is_vipt_aliasing()) {
while (CACHE_COLOUR((vma->vm_start ^
(uint32_t)proc->buffer))) {
Should this be (uintptr_t)?
/*pr_info("binder_mmap: %d %lx-%lx maps %p\n",
Delete the code, don't comment it out. It is on more than one place.
static void print_binder_thread(struct seq_file *m,
struct binder_thread *thread,
int print_always)
{
struct binder_transaction *t;
struct binder_work *w;
size_t start_pos = m->count;
size_t header_pos;
seq_printf(m, " thread %d: l %02x\n", thread->pid,
thread->looper);
header_pos = m->count;
t = thread->transaction_stack;
while (t) {
if (t->from == thread) {
print_binder_transaction(m,
"
outgoing transaction", t);
t = t->from_parent;
Is anyone depending on the debugfs files? Can it be deleted?
Code indentation is "interesting" in binder_thread_read(). See the "}
break;" lines. {}s should not be needed...?
I don't think this code would get merged if it was submitted for
normal inclusion in kernel. I don't think it is good idea to push it
through the back door, without documenting what it does and without
patches even going to the lists.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: binder: move to the "real" part of the kernel
On Mon, Oct 20, 2014 at 06:04:50AM +0800, Greg Kroah-Hartman wrote: > There is now work to resolve the interface, it requires someone who has > the rights to push to Android userspace. But that is going to be a > "total rewrite", and until then, this code needs to be used, no matter > how much we hate this. It helps to qualify why it absolutely has to, and why this is different from other interfaces we haven't merged. Is this the last building block to run upstream Linux on a common Android device out of the box without needing any patches or out of tree drivers? Is there any other good reason I might have missed. To convince other people that merging a piece like this absolutely needs to get merged I'd suggest you start with presenting factual argument, and then let the broader audience weight their merrits. I think with the known problems of the code, and the fact that the real user ABI is a library anyway the stakes are quite high here. So as a start please prepare a list of arguments, a detailed description of the ABI, and post a proper patch (not a move) that suggests adding this driver to all the relevant lists (most importantly linux-fsdevel and linux-api) so that people with the right experience can review it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] tools: hv: fcopy_daemon: Unused variable removed
From: Matej Mužila
Remove unused variable
Signed-off-by: Matej Mužila
Acked-by: K. Y. Srinivasan
---
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index 6f27e2f..1fc2dc2 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -132,7 +132,7 @@ static int hv_copy_cancel(void)
int main(void)
{
- int fd, fcopy_fd, len;
+ int fcopy_fd, len;
int error;
int version = FCOPY_CURRENT_VERSION;
char *buffer[4096 * 2];
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] tools: hv: fcopy_daemon: Check buffer limits
From: Matej Mužila
Check if cpmsg->size is in limits of DATA_FRAGMENT
Signed-off-by: Matej Mužila
Acked-by: K. Y. Srinivasan
---
If corrupted data are read from /dev/vmbus/hv_fcopy, pwrite can
read from memory outside of the buffer (defined at line 138).
Added check.
---
@@ -104,6 +104,10 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
{
ssize_t bytes_written;
+ // Check if the cpmsg->size is in limits of DATA_FRAGMENT
+ if (cpmsg->size > DATA_FRAGMENT * sizeof(__u8))
+ return HV_E_FAIL;
+
bytes_written = pwrite(target_fd, cpmsg->data, cpmsg->size,
cpmsg->offset);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] tools: hv: fcopy_daemon: Don't use uninitialized variable
From: Matej Mužila
Don't use uninitialized variable
Signed-off-by: Matej Mužila
Acked-by: K. Y. Srinivasan
---
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index 1fc2dc2..0f8f918 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -191,7 +191,7 @@ int main(void)
default:
syslog(LOG_ERR, "Unknown operation: %d",
in_msg->operation);
-
+ error = HV_ERROR_NOT_SUPPORTED;
}
if (pwrite(fcopy_fd, &error, sizeof(int), 0) != sizeof(int)) {
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] tools: hv: fcopy_daemon: Check buffer limits
On Tue, 21 Oct 2014 13:49:00 +0200
Matej Mužila wrote:
> From: Matej Mužila
>
> Check if cpmsg->size is in limits of DATA_FRAGMENT
>
> Signed-off-by: Matej Mužila
> Acked-by: K. Y. Srinivasan
> ---
> If corrupted data are read from /dev/vmbus/hv_fcopy, pwrite can
> read from memory outside of the buffer (defined at line 138).
> Added check.
> ---
> @@ -104,6 +104,10 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
> {
> ssize_t bytes_written;
>
> + // Check if the cpmsg->size is in limits of DATA_FRAGMENT
> + if (cpmsg->size > DATA_FRAGMENT * sizeof(__u8))
> + return HV_E_FAIL;
> +
- C style comments for coding style, also sizeof(__u8) is by definition 1
so it's perhaps surplus ?
Also your patch block is devoid of a few thins like the file name...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: linux-3.18-rc1 bug report
On 10/21/2014 07:33 AM, David Binderman wrote:
Hello there,
1.
[linux-3.18-rc1/drivers/staging/dgap/dgap.c:6692]: (warning) Logical
disjunction always evaluates to true: conc_type != 65 || conc_type != 66.
Source code is
if (conc_type == 0 || conc_type != CX ||
conc_type != EPC) {
2.
[linux-3.18-rc1/drivers/staging/dgap/dgap.c:6733]: (warning) Logical
disjunction always evaluates to true: module_type != 68 || module_type != 73.
if (module_type == 0 || module_type != PORTS ||
module_type != MODEM) {
Suggest code rework in both cases
Regards
David Binderman
Yes, this is certainly wrong. Eventually, all that code parsing the
config file will have to be removed. Those above 2 sections particularly
because I don't see any way to support the concentrators at the moment.
Thanks
Mark
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] tools: hv: fcopy_daemon: Check buffer limits
> sizeof(__u8) is by definition 1 so it's perhaps surplus ?
Now the size is now determined from the structure definition in
include/uapi/linux/hyperv.h
> - C style comments for coding style
Fixed
> Also your patch block is devoid of a few thins like the file name...
I'm sorry, the (missing) filename mistake occured in copy-paste process.
Here is the patch as it (I hope) should look like:
---
From: Matej Mužila
Check if cpmsg->size is in limits of DATA_FRAGMENT
Signed-off-by: Matej Mužila
---
If corrupted data are read from /dev/vmbus/hv_fcopy, pwrite can
read from memory outside of the buffer (defined at line 138).
Added check.
---
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index 6f27e2f..1fc2dc2 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -104,6 +104,10 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
{
ssize_t bytes_written;
+ /* Check if the cpmsg->size is in limits of DATA_FRAGMENT */
+ if (cpmsg->size > sizeof(cpmsg->data))
+ return HV_E_FAIL;
+
bytes_written = pwrite(target_fd, cpmsg->data, cpmsg->size,
cpmsg->offset);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: binder: move to the "real" part of the kernel
On Tuesday 21 October 2014 12:36:22 Pavel Machek wrote:
> On Fri 2014-10-17 01:12:21, Greg Kroah-Hartman wrote:
> > On Thu, Oct 16, 2014 at 10:09:04AM -0700, John Stultz wrote:
> > > On Thu, Oct 16, 2014 at 5:47 AM, Greg Kroah-Hartman
> > > wrote:
> > > > From: Greg Kroah-Hartman
>
> > > Are the Android guys comfortable with the ABI stability rules they'll
> > > now face?
> >
> > Just because something is in staging, doesn't mean you don't have to
> > follow the same ABI stability rules as the rest of the kernel. If a
> > change had happened to this code that broke userspace in the past, I
> > would have reverted it. So this should not be anything different from
> > what has been happening inthe past.
>
> Actually, there's big difference.
>
> If Al Viro changes core filesystem in a way that breaks
> staging/binder, binder is broken, and if it can't be fixed... well it
> can't be fixed.
>
> If Al Viro changes core filesystem in a way that breaks
> drivers/binder, Al's change is going to be reverted.
One might have argued that we'd have to do that already, but the reasons
for doing that with binder in the main kernel are certainly stronger.
> It is really hard to review without API documentation. Normally, API
> documentation is required for stuff like this.
>
> For example: does it add new files in /proc?
>
> Given that it is stable, can we get rid of binder_debug() and
> especially BINDER_DEBUG_ENTRY stuff?
Good point. We require documentation for every single sysfs attribute
that gets added to a driver (some escape the review, but that doesn't
change the rule), so we should not make an exception for a new procfs
file here.
> Checkpatch warns about 98 too long lines. Some of them could be fixed
> easily.
I don't think this should be an argument.
> This looks scary:
>
> trace_binder_transaction_fd(t, fp->handle,
> target_fd);
> binder_debug(BINDER_DEBUG_TRANSACTION,
> "fd %d -> %d\n",
> fp->handle, target_fd);
> /* TODO: fput? */
> fp->handle = target_fd;
> } break;
>
> Could binder_transcation() be split to smaller functions according to
> CodingStyle? 17 goto targets at the end of function are not exactly
> easy to read.
>
> ginder_thread_read/write also needs splitting.
Yes, in principle, but this is still a detail that would mainly serve
to simplify review. The problem is more the lack of review and
documentation of the API.
> binder_ioctl_write_read: just use direct return, no need to goto out
> if it just returns.
details, and a lot of people actually like the style used there.
>proc->user_buffer_offset = vma->vm_start - (uintptr_t)proc->buffer;
> mutex_unlock(&binder_mmap_lock);
>
> #ifdef CONFIG_CPU_CACHE_VIPT
> if (cache_is_vipt_aliasing()) {
> while (CACHE_COLOUR((vma->vm_start ^
> (uint32_t)proc->buffer))) {
>
> Should this be (uintptr_t)?
It should probably call an architecture specific helper.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] tools: hv: fcopy_daemon: Check buffer limits
On Tue, Oct 21, 2014 at 02:59:58PM +0200, Matej Mužila wrote: > > sizeof(__u8) is by definition 1 so it's perhaps surplus ? > Now the size is now determined from the structure definition in > include/uapi/linux/hyperv.h > > > - C style comments for coding style > Fixed > > > Also your patch block is devoid of a few thins like the file name... > I'm sorry, the (missing) filename mistake occured in copy-paste process. > Copy and paste is very error prone... > > Here is the patch as it (I hope) should look like: This patch looks good, but please resend it as a proper v2 patch. https://www.google.com/search?q=how+to+send+a+v2+patch > --- > From: Matej Mužila > > Check if cpmsg->size is in limits of DATA_FRAGMENT > > Signed-off-by: Matej Mužila > --- > If corrupted data are read from /dev/vmbus/hv_fcopy, pwrite can > read from memory outside of the buffer (defined at line 138). > Added check. Put this information in the patch description and not beyond the cut off. That information is useful. The cut off is meant for meta comentary to say what changed between v1 and v2 etc, which is nice to have but we don't want to preserve it. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: skein: Loadable Module Support
Adds loadable module support for Skein256, Skein512, and Skein1024 Hash Algorithms. Signed-off-by: Eric Rost --- drivers/staging/skein/Kconfig | 12 + drivers/staging/skein/Makefile| 6 +++ drivers/staging/skein/skein.c | 11 +++- drivers/staging/skein/skein.h | 6 +++ drivers/staging/skein/skein1024_generic.c | 88 +++ drivers/staging/skein/skein256_generic.c | 88 +++ drivers/staging/skein/skein512_generic.c | 88 +++ 7 files changed, 298 insertions(+), 1 deletion(-) create mode 100644 drivers/staging/skein/skein1024_generic.c create mode 100644 drivers/staging/skein/skein256_generic.c create mode 100644 drivers/staging/skein/skein512_generic.c diff --git a/drivers/staging/skein/Kconfig b/drivers/staging/skein/Kconfig index b9172bf..e260111 100644 --- a/drivers/staging/skein/Kconfig +++ b/drivers/staging/skein/Kconfig @@ -15,6 +15,18 @@ config CRYPTO_SKEIN for more information. This module depends on the threefish block cipher module. +config CRYPTO_SKEIN_256 + tristate "Skein256 driver" + select CRYPTO_SKEIN + +config CRYPTO_SKEIN_512 + tristate "Skein512 driver" + select CRYPTO_SKEIN + +config CRYPTO_SKEIN_1024 + tristate "Skein1024 driver" + select CRYPTO_SKEIN + config CRYPTO_THREEFISH bool "Threefish tweakable block cipher" depends on (X86 || UML_X86) && 64BIT && CRYPTO diff --git a/drivers/staging/skein/Makefile b/drivers/staging/skein/Makefile index a14aadd..1be01fe 100644 --- a/drivers/staging/skein/Makefile +++ b/drivers/staging/skein/Makefile @@ -5,5 +5,11 @@ obj-$(CONFIG_CRYPTO_SKEIN) += skein.o \ skein_api.o \ skein_block.o +obj-$(CONFIG_CRYPTO_SKEIN_256) += skein256_generic.o + +obj-$(CONFIG_CRYPTO_SKEIN_512) += skein512_generic.o + +obj-$(CONFIG_CRYPTO_SKEIN_1024) += skein1024_generic.o + obj-$(CONFIG_CRYPTO_THREEFISH) += threefish_block.o \ threefish_api.o diff --git a/drivers/staging/skein/skein.c b/drivers/staging/skein/skein.c index 8cc8358..2138e22 100644 --- a/drivers/staging/skein/skein.c +++ b/drivers/staging/skein/skein.c @@ -11,6 +11,7 @@ #define SKEIN_PORT_CODE /* instantiate any code in skein_port.h */ #include/* get the memcpy/memset functions */ +#include #include "skein.h" /* get the Skein API definitions */ #include "skein_iv.h"/* get precomputed IVs */ #include "skein_block.h" @@ -73,6 +74,7 @@ int skein_256_init(struct skein_256_ctx *ctx, size_t hash_bit_len) return SKEIN_SUCCESS; } +EXPORT_SYMBOL(skein_256_init); /**/ /* init the context for a MAC and/or tree hash operation */ @@ -191,7 +193,7 @@ int skein_256_update(struct skein_256_ctx *ctx, const u8 *msg, return SKEIN_SUCCESS; } - +EXPORT_SYMBOL(skein_256_update); /**/ /* finalize the hash computation and output the result */ int skein_256_final(struct skein_256_ctx *ctx, u8 *hash_val) @@ -240,6 +242,7 @@ int skein_256_final(struct skein_256_ctx *ctx, u8 *hash_val) } return SKEIN_SUCCESS; } +EXPORT_SYMBOL(skein_256_final); /*/ /* 512-bit Skein */ @@ -303,6 +306,7 @@ int skein_512_init(struct skein_512_ctx *ctx, size_t hash_bit_len) return SKEIN_SUCCESS; } +EXPORT_SYMBOL(skein_512_init); /**/ /* init the context for a MAC and/or tree hash operation */ @@ -420,6 +424,7 @@ int skein_512_update(struct skein_512_ctx *ctx, const u8 *msg, return SKEIN_SUCCESS; } +EXPORT_SYMBOL(skein_512_update); /**/ /* finalize the hash computation and output the result */ @@ -469,6 +474,7 @@ int skein_512_final(struct skein_512_ctx *ctx, u8 *hash_val) } return SKEIN_SUCCESS; } +EXPORT_SYMBOL(skein_512_final); /*/ /*1024-bit Skein */ @@ -526,6 +532,7 @@ int skein_1024_init(struct skein_1024_ctx *ctx, size_t hash_bit_len) return SKEIN_SUCCESS; } +EXPORT_SYMBOL(skein_1024_init); /**/ /* init the context for a MAC and/or tree hash operation */ @@ -644,6 +651,7 @@ int skein_1024_update(struct skein_1024_ctx *ctx, const u8 *msg, return SKEIN_SUCCESS; } +EXPORT_SYMBOL(skein_1024_update); /**/ /* finalize the hash computation and output the result */ @@ -693,6 +701,7 @@ int s
[PATCH v2 1/3] tools: hv: fcopy_daemon: Check buffer limits
From: Matej Mužila
Check if cpmsg->size is in limits of DATA_FRAGMENT
Signed-off-by: Matej Mužila
---
If corrupted data are read from /dev/vmbus/hv_fcopy, pwrite can
read from memory outside of the buffer (defined at line 138).
Added check.
Changes made since v1:
* max value of cmesg->size is now derived from structure
definition in sources/include/uapi/linux/hyperv.h
* Fixed comments
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index 6f27e2f..1fc2dc2 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -104,6 +104,10 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
{
ssize_t bytes_written;
+ /* Check if the cpmsg->size is in limits of DATA_FRAGMENT */
+ if (cpmsg->size > sizeof(cpmsg->data))
+ return HV_E_FAIL;
+
bytes_written = pwrite(target_fd, cpmsg->data, cpmsg->size,
cpmsg->offset);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8712: Remove redundant cast
struct firmware::data has type const u8*, as does *ppmappedfw, so the cast to u8* is unnecessary and slightly confusing. Signed-off-by: Rasmus Villemoes --- drivers/staging/rtl8712/hal_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c index 1d6ade0..cc6b390 100644 --- a/drivers/staging/rtl8712/hal_init.c +++ b/drivers/staging/rtl8712/hal_init.c @@ -86,7 +86,7 @@ static u32 rtl871x_open_fw(struct _adapter *padapter, const u8 **ppmappedfw) (int)padapter->fw->size); return 0; } - *ppmappedfw = (u8 *)((*praw)->data); + *ppmappedfw = ((*praw)->data); return (*praw)->size; } -- 2.0.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6655: Remove redundant cast
Both sides have type const struct iw_handler_def*, so the cast is
unnecessary and confusing.
Signed-off-by: Rasmus Villemoes
---
drivers/staging/vt6655/device_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vt6655/device_main.c
b/drivers/staging/vt6655/device_main.c
index 54e16f4..9281713 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -936,7 +936,7 @@ vt6655_probe(struct pci_dev *pcid, const struct
pci_device_id *ent)
dev->irq= pcid->irq;
dev->netdev_ops = &device_netdev_ops;
- dev->wireless_handlers = (struct iw_handler_def *)&iwctl_handler_def;
+ dev->wireless_handlers = &iwctl_handler_def;
rc = register_netdev(dev);
if (rc) {
--
2.0.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: Remove redundant cast
On Tue, Oct 21, 2014 at 04:58:26PM +0200, Rasmus Villemoes wrote: > struct firmware::data has type const u8*, as does *ppmappedfw, so the > cast to u8* is unnecessary and slightly confusing. > > Signed-off-by: Rasmus Villemoes > --- > drivers/staging/rtl8712/hal_init.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8712/hal_init.c > b/drivers/staging/rtl8712/hal_init.c > index 1d6ade0..cc6b390 100644 > --- a/drivers/staging/rtl8712/hal_init.c > +++ b/drivers/staging/rtl8712/hal_init.c > @@ -86,7 +86,7 @@ static u32 rtl871x_open_fw(struct _adapter *padapter, const > u8 **ppmappedfw) > (int)padapter->fw->size); > return 0; > } > - *ppmappedfw = (u8 *)((*praw)->data); > + *ppmappedfw = ((*praw)->data); Remove the unneeded parens as well, please. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: addi_apci_035: remove driver
On 20/10/14 18:37, H Hartley Sweeten wrote: According to ADDI-DATA, this board was discontinued last year and they feel that no further development is needed for this driver. Remove the driver from comedi to help with the addi-data cleanup. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/Kconfig | 8 - drivers/staging/comedi/drivers/Makefile| 1 - .../comedi/drivers/addi-data/hwdrv_apci035.c | 482 - drivers/staging/comedi/drivers/addi_apci_035.c | 129 -- 4 files changed, 620 deletions(-) delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c delete mode 100644 drivers/staging/comedi/drivers/addi_apci_035.c An alternative may be to convert it into a very simple driver by stripping out all the interrupt, timer and watchdog stuff, and just leave an AI subdevice just with insn_read support. But even that seems hopelessly broken in this driver - it doesn't seem to bother selecting the channel or the range, for example. So I've no qualms about removing it altogether. Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_65xx: remove deadcode in ni_65xx_intr_cmdtest()
On 20/10/14 23:02, H Hartley Sweeten wrote: Reported by: coverity (CID 142963) Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/ni_65xx.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_65xx.c b/drivers/staging/comedi/drivers/ni_65xx.c index 195b10b..bcb326e 100644 --- a/drivers/staging/comedi/drivers/ni_65xx.c +++ b/drivers/staging/comedi/drivers/ni_65xx.c @@ -534,9 +534,6 @@ static int ni_65xx_intr_cmdtest(struct comedi_device *dev, /* Step 2a : make sure trigger sources are unique */ /* Step 2b : and mutually compatible */ - if (err) - return 2; - /* Step 3: check if arguments are trivially valid */ err |= cfc_check_trigger_arg_is(&cmd->start_arg, 0); Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_65xx: remove deadcode in ni_65xx_intr_cmdtest()
On 21/10/14 16:14, Ian Abbott wrote: On 20/10/14 23:02, H Hartley Sweeten wrote: Reported by: coverity (CID 142963) Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/ni_65xx.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_65xx.c b/drivers/staging/comedi/drivers/ni_65xx.c index 195b10b..bcb326e 100644 --- a/drivers/staging/comedi/drivers/ni_65xx.c +++ b/drivers/staging/comedi/drivers/ni_65xx.c @@ -534,9 +534,6 @@ static int ni_65xx_intr_cmdtest(struct comedi_device *dev, /* Step 2a : make sure trigger sources are unique */ /* Step 2b : and mutually compatible */ -if (err) -return 2; - /* Step 3: check if arguments are trivially valid */ err |= cfc_check_trigger_arg_is(&cmd->start_arg, 0); Reviewed-by: Ian Abbott Although there ought to be a dash in your "Reported by:" line, Hartley. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_6527: remove deadcode in ni6527_intr_cmdtest()
On 20/10/14 23:03, H Hartley Sweeten wrote: Reported by: coverity (CID 142962) There should be a dash in "Reported by". Not sure if it matters. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/ni_6527.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_6527.c b/drivers/staging/comedi/drivers/ni_6527.c index 1a62b4f..f99847f 100644 --- a/drivers/staging/comedi/drivers/ni_6527.c +++ b/drivers/staging/comedi/drivers/ni_6527.c @@ -237,9 +237,6 @@ static int ni6527_intr_cmdtest(struct comedi_device *dev, /* Step 2a : make sure trigger sources are unique */ /* Step 2b : and mutually compatible */ - if (err) - return 2; - /* Step 3: check if arguments are trivially valid */ err |= cfc_check_trigger_arg_is(&cmd->start_arg, 0); Other than the issue with the "Reported by" line... Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_mio_common: remove deadcode in ni_cdio_cmdtest()
On 20/10/14 23:04, H Hartley Sweeten wrote: Reported by: coverity (CID 142967) Should be a dash in "Reported by". Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/ni_mio_common.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 4a1d0ba..4672a14 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -3490,9 +3490,6 @@ static int ni_cdio_cmdtest(struct comedi_device *dev, /* Step 2a : make sure trigger sources are unique */ /* Step 2b : and mutually compatible */ - if (err) - return 2; - /* Step 3: check if arguments are trivially valid */ err |= cfc_check_trigger_arg_is(&cmd->start_arg, 0); Other than the issue on the "Reported by" line... Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
*****SPAM***** 2ND NOTICE AGAIN ,
Our MailScanner believes that the attachment to this message sent to you From: stafdmi...@flametv.co.uk Subject: 2ND NOTICE AGAIN , is Unsolicited Commercial Email (spam). Unless you are sure that this message is incorrectly thought to be spam, please delete this message without opening it. Opening spam messages might allow the spammer to verify your email address. If you believe that this message has been incorrectly marked as spam, please forward this email to postmaster. Date: 20141021 pts rule name description -- -- -1.4 ALL_TRUSTEDPassed through trusted hosts only via SMTP 0.0 MISSING_MIDMissing Message-Id: header 1.8 SUBJ_ALL_CAPS Subject is all capitals 1.6 MISSING_HEADERSMissing To: header 4.2 FORGED_MUA_OUTLOOK Forged mail pretending to be from MS Outlook --- Begin Message --- Greetings, I understand that through Internet is not the best way to link up with you because of the confidentiality which my proposal demands. However, I have already sent you this same letter one month ago,but I am not sure if it did get to you since I have not heard from you, hence i am constrain to reach you through the Internet which has been abused over the years. I wish to notify you again that You were listed as a Heir to the total sum of (Three Million Six Hundred Thousand British Pounds) in the codicil and last testament of the deceased.(Name now withheld since this is our second letter to you). We contacted you because you bear the surname identity and therefore can present you as the Heir to the inheritance funds. Please indicate your interest immediately for us to proceed. I shall feed you with full details of this transaction upon receipt of your reply towards this proposal. All the legal papers will be processed in your acceptance. In your acceptance, we request that you kindly forward to us your letter of acceptance; your current telephone and fax numbers and a forwarding address to enable us file necessary documents at our high court probate division for the release of this sum of money. I look forward to hearing from you. Yours truly, Admin Staff. Will Drafters Ltd --- End Message --- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/9] staging: comedi: addi_apci_1564: additional clean up
On 20/10/14 18:57, H Hartley Sweeten wrote: This driver currently passes the timer "channel" in a manner that violates the comedi API. Fix the timer subdevice so that the timers "channels" are correctly used from the insn->chanspec. Fix the I/O access for the boards registers. Currently the iobase used to access the counters and the main board registers are incorrect. According to the ADDI-DATA datasheet the PCI BARs are: PCI BAR0 - amcc chip registers PCI BAR1 - main board registers PCI BAR2 - counter registers Move the register map defines from the included hwdrv_apci1564.c file to the main driver source file. H Hartley Sweeten (9): staging: comedi: addi_apci_1564: remove APCI1564_COUNTER[1234] defines staging: comedi: addi_apci_1564: remove private data 'mode_select_register' staging: comedi: addi_apci_1564: fix counter register access staging: comedi: addi_apci_1564: board has 4 timers staging: comedi: addi_apci_1564: fix board register access staging: comedi: addi_apci_1564: tidy up private data 'amcc_iobase' staging: comedi: addi_apci_1564: move register map defines to driver staging: comedi: addi_apci_1564: move APCI1564_DI_IRQ_REG bit defines staging: comedi: addi_apci_1564: move APCI1564_DO_INT_CTRL/STATUS_REG bit defines .../comedi/drivers/addi-data/hwdrv_apci1564.c | 155 ++ drivers/staging/comedi/drivers/addi_apci_1564.c| 179 + 2 files changed, 159 insertions(+), 175 deletions(-) The changes to PCI BARs worry me. Perhaps the old PCI BARs worked with older models of the card. Presumably they stopped using the AMCC chip some years ago as it's no longer available. They might have moved the registers around and changed the PCI IDs at the same time, or possibly moved the registers around and *not* changed the PCI IDs! (That nearly happened with the MeasurementComputing/ComputerBoards PCI-DIO48H which is now in the comedi 8255_pci driver, except it did turn out that they changed the PCI subdevice IDs after switching from the AMCC S5933 (I think) to a PLX PCI9050/PCI9052.) Well, ADDI-DATA have their own Linux driver for this board which only seems to use BAR0 and BAR1, so something fishy is going on. http://www.addi-data.com/treiber/linux/kernel_ioctl_api/apci1564_r1283.tar.bz2 -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/9] staging: comedi: addi_apci_1564: additional clean up
On Tuesday, October 21, 2014 9:29 AM, Ian Abbott wrote: > On 20/10/14 18:57, H Hartley Sweeten wrote: >> This driver currently passes the timer "channel" in a manner that violates >> the comedi API. Fix the timer subdevice so that the timers "channels" are >> correctly used from the insn->chanspec. >> >> Fix the I/O access for the boards registers. Currently the iobase used to >> access the counters and the main board registers are incorrect. According >> to the ADDI-DATA datasheet the PCI BARs are: >> >>PCI BAR0 - amcc chip registers >>PCI BAR1 - main board registers >>PCI BAR2 - counter registers >> >> Move the register map defines from the included hwdrv_apci1564.c file to >> the main driver source file. >> >> H Hartley Sweeten (9): >>staging: comedi: addi_apci_1564: remove APCI1564_COUNTER[1234] defines >>staging: comedi: addi_apci_1564: remove private data >> 'mode_select_register' >>staging: comedi: addi_apci_1564: fix counter register access >>staging: comedi: addi_apci_1564: board has 4 timers >>staging: comedi: addi_apci_1564: fix board register access >>staging: comedi: addi_apci_1564: tidy up private data 'amcc_iobase' >>staging: comedi: addi_apci_1564: move register map defines to driver >>staging: comedi: addi_apci_1564: move APCI1564_DI_IRQ_REG bit defines >>staging: comedi: addi_apci_1564: move APCI1564_DO_INT_CTRL/STATUS_REG bit >> defines >> >> .../comedi/drivers/addi-data/hwdrv_apci1564.c | 155 ++ >> drivers/staging/comedi/drivers/addi_apci_1564.c| 179 >> + >> 2 files changed, 159 insertions(+), 175 deletions(-) >> > > The changes to PCI BARs worry me. Perhaps the old PCI BARs worked with > older models of the card. Presumably they stopped using the AMCC chip > some years ago as it's no longer available. They might have moved the > registers around and changed the PCI IDs at the same time, or possibly > moved the registers around and *not* changed the PCI IDs! (That nearly > happened with the MeasurementComputing/ComputerBoards PCI-DIO48H which > is now in the comedi 8255_pci driver, except it did turn out that they > changed the PCI subdevice IDs after switching from the AMCC S5933 (I > think) to a PLX PCI9050/PCI9052.) > > Well, ADDI-DATA have their own Linux driver for this board which only > seems to use BAR0 and BAR1, so something fishy is going on. > > http://www.addi-data.com/treiber/linux/kernel_ioctl_api/apci1564_r1283.tar.bz2 Ian, The I/O Mapping document I have from ADDI-DATA is dated 27.02.2004. It was modified on 29.07.08 with the counter information. The ADD-DATA driver only using PCI BARS 0 and 1 makes sense if that driver does not have any counter support. As messed up as the ADDI-DATA drivers were, my guess is the PCI BAR issue was initially a screw up when the drivers were initially merged. I will ask ADDI-DATA about the PCI BARs to see if they have any additional information. Regards, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send()
Thanks Sitsofe. This should have been fixed by this patch: http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=f88e67149f97d73c704d6fe6f492edde97463025 Can you give it a try? From: Sitsofe Wheeler Sent: Saturday, October 11, 2014 3:36 AM To: Long Li Cc: David Miller; o...@aepfle.de; net...@vger.kernel.org; jasow...@redhat.com; linux-ker...@vger.kernel.org; a...@canonical.com; de...@linuxdriverproject.org Subject: Re: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send() On Fri, Oct 10, 2014 at 11:39:00PM +, Long Li wrote: > Thanks Sitsofe. Can you provide more details on the test setup? > > The kernel trace shows that skb->mac_header=0x (which means not > yet set, it's in RCX: ). See reply below. > -Original Message- > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On Behalf > Of Sitsofe Wheeler > Sent: Thursday, October 09, 2014 6:32 AM > To: David Miller > Cc: o...@aepfle.de; net...@vger.kernel.org; jasow...@redhat.com; > linux-ker...@vger.kernel.org; a...@canonical.com; > de...@linuxdriverproject.org Subject: Re: [PATCH net 1/1] hyperv: Fix > a bug in netvsc_send() > > On Sun, Oct 05, 2014 at 09:11:29PM -0400, David Miller wrote: > > From: "K. Y. Srinivasan" > > Date: Sun, 5 Oct 2014 10:42:51 -0700 > > > > > After the packet is successfully sent, we should not touch the > > > packet as it may have been freed. This patch is based on the work > > > done by Long Li . > > > > > > David, please queue this up for stable. > > With 3.17.0 g782d59c (which should include this patch) I'm still seeing the > following: > > Oct 09 13:14:51 a dhclient[538]: DHCPREQUEST on eth0 to 255.255.255.255 port > 67 (xid=0x1dd33078) Oct 09 13:14:51 a dhclient[538]: DHCPACK from 10.x.x.x > (xid=0x1dd33078) Oct 09 13:14:55 a kernel: BUG: unable to handle kernel > paging request at 8800ed2e72e3 Oct 09 13:14:55 a kernel: IP: > [] netvsc_select_queue+0x3d/0x150 Oct 09 13:14:55 a kernel: > PGD 2db5067 PUD 2075be067 PMD 207454067 PTE 8000ed2e7060 Oct 09 13:14:55 > a kernel: Oops: [#1] SMP DEBUG_PAGEALLOC Oct 09 13:14:55 a kernel: CPU: > 6 PID: 566 Comm: arping Not tainted 3.17.0.x86_64-05585-g782d59c #147 Oct 09 > 13:14:55 a kernel: Hardware name: Microsoft Corporation Virtual > Machine/Virtual Machine, BIOS 090006 05/23/2012 Oct 09 13:14:55 a kernel: > task: 8801f978b9f0 ti: 8801f3b84000 task.ti: 8801f3b84000 Oct 09 > 13:14:55 a kernel: RIP: 0010:[] [] > netvsc_select_queue+0x3d/0x150 Oct 09 13:14:55 a kernel: RSP: > 0018:8801f3b87c60 EFLAGS: 00010202 Oct 09 13:14:55 a kernel: RAX: RBX: 8800f13e8000 RCX: Oct 09 13:14:55 a kernel: RDX: 8800ed2d72d8 RSI: 8801fabca1c0 RDI: 8800f13e8000 Oct 09 13:14:55 a kernel: RBP: 8801f3b87c88 R08: 002a R09: Oct 09 13:14:55 a kernel: R10: 8801f83b3f60 R11: 0008 R12: 8801fabca1c0 Oct 09 13:14:55 a kernel: R13: R14: 8800ed359bd8 R15: 8801fabca1c0 Oct 09 13:14:55 a kernel: FS: 7f943a5c9740() GS:880206cc() knlGS: Oct 09 13:14:55 a kernel: CS: 0010 DS: ES: CR0: 80050033 Oct 09 13:14:55 a kernel: CR2: 8800ed2e72e3 CR3: 0001f3957000 CR4: 000406e0 Oct 09 13:14:55 a kernel: Stack: I'm not quite sure what's happened to the formatting above but oh well. My kernel config is essentially the same as the one described in https://lkml.org/lkml/2014/8/19/708 but the key might be that it is configured to do extra verification setup/checks. The guest is currently configured to have 8 vpcus and 8GBytes of RAM. # # Automatically generated file; DO NOT EDIT. # Linux/x86 3.17.0 Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_HAVE_LATENCYTOP_SUPPORT=y CONFIG_MMU=y CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ZONE_DMA32=y CONFIG_AUDIT_ARCH=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_HAVE_INTEL_TXT=y CONFIG_X86_64_SMP=y CONFIG_X86_HT=y CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fca
Re: [PATCH 0/9] staging: comedi: addi_apci_1564: additional clean up
On Tuesday, October 21, 2014 9:29 AM, Ian Abbott wrote: > On 20/10/14 18:57, H Hartley Sweeten wrote: >> This driver currently passes the timer "channel" in a manner that violates >> the comedi API. Fix the timer subdevice so that the timers "channels" are >> correctly used from the insn->chanspec. >> >> Fix the I/O access for the boards registers. Currently the iobase used to >> access the counters and the main board registers are incorrect. According >> to the ADDI-DATA datasheet the PCI BARs are: >> >>PCI BAR0 - amcc chip registers >>PCI BAR1 - main board registers >>PCI BAR2 - counter registers Ian, Looking over the current driver I think the BAR mapping above is correct. There are a number of inconsistencies in the driver right now. In apci1564_interrupt() the devpriv->amcc_iobase (BAR0) is used to access one of the AMCC registers: /* check interrupt is from this device */ if ((inl(devpriv->amcc_iobase + AMCC_OP_REG_INTCSR) & INTCSR_INTR_ASSERTED) == 0) return IRQ_NONE; That seems to indicate that the AMCC PCI registers are mapped to BAR0, which matches the AMCC datasheet. That BAR could then not be used to access the boards registers. In addition, in apci1564_interrupt() the dev->iobase (BAR1) is used for: s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG) & 0x; But is apci1564_reset() and apci1564_cos_insn_config() that register is accessed using devpriv->amcc_iobase (BAR0): inl(devpriv->amcc_iobase + APCI1564_DI_INT_STATUS_REG); The register should not be accessible from both BARs so BAR1 must be the real base address for the boards main registers. dev->iobase (BAR1) is currently used to access the counter register (other than the one access to APCI1564_DI_INT_STATUS_REG listed above). The mapping of those registers would overlay the boards main registers so my guess is they should really use BAR2 as the base address. I need to dig thru the git history to see if the screw up was caused by various patches to the driver or if the problem existed from when it was initially merged. I have sent an email to ADDI-DATA asking for clarification. Regards, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: dgap: re-arrange functions for removing forward declarations.
On 10/14/2014 08:01 AM, Mark Hounschell wrote: On 10/13/2014 10:04 PM, Greg KH wrote: On Mon, Oct 13, 2014 at 07:56:38AM -0700, Joe Perches wrote: On Mon, 2014-10-13 at 17:01 +0900, DaeSeok Youn wrote: Hi, 2014-10-13 12:25 GMT+09:00 Greg KH : On Mon, Oct 13, 2014 at 11:34:25AM +0900, Daeseok Youn wrote: Re-arrange the functions for removing forward declarations. Signed-off-by: Daeseok Youn --- This patch has too many changes for re-arranging the functions. So I wonder that I should break this up into smaller patches. Are the .o files identical before and after this patch? If so, it's fine. Ok. I will check for that. The .o files shouldn't be identical after function reordering. Hm, they might be the same size, but I can see how on some architectures (like ppc) how that would not be the case, you are right. Isn't there an "objdiff" program or something like that which might help in validating that nothing "changed" in the source for type of patch that just moves functions around in a file. thanks, Greg, Would just testing the thing be of any help? Regards Mark I don't know what is going on with this patch, but for what it's worth, I have applied this patch and my testing still shows everything OK. Tested-by: Mark Hounschell Regards Mark ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks
> -Original Message-
> From: Sitsofe Wheeler [mailto:sits...@gmail.com]
> Sent: Monday, October 20, 2014 9:46 PM
> To: KY Srinivasan
> Cc: Jeff Leung; James Bottomley; Christoph Hellwig; Haiyang Zhang; Christoph
> Hellwig; Hannes Reinecke; linux-s...@vger.kernel.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org
> Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks
>
> On Sun, Oct 12, 2014 at 01:21:01AM +, KY Srinivasan wrote:
> >
> > > -Original Message-
> > > From: Jeff Leung [mailto:jle...@v10networks.ca]
> > > Sent: Saturday, October 11, 2014 1:22 PM
> > >
> > > > On the current release of Windows (windows 10), we are advertising
> > > > SPC3 compliance.
> > > > We are ok with declaring compliance to SPC3 in our drivers.
> > > If you are going to declare SPC3 compliance in the drivers, are you
> > > going to put in checks to ensure that SPC-3 compliance doesn't get
> > > accidentally enabled for hypervisors below Win10?
> > >
> > > I do know for a fact that Ubuntu's kernels already force SPC3
> > > compliance to enable specific features such as TRIM on earlier
> > > versions of Hyper-V (Namely Hyper-V 2012 and 2012 R2).
> > You are right; Ubuntu has been carrying a patch that was doing just
> > this and this has been working without any issues on many earlier
> > versions of Windows. (2012 and 2012 R2). On windows 10 we don't need
> > any changes in the Linux driver as the host itself is advertising SPC3
> > compliance. Based on the testing we have done with Ubuntu, we are
> > comfortable picking up that patch.
>
> OK this seems to be the patch currently carried by Ubuntu:
>
> From ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d Mon Sep 17 00:00:00 2001
> From: Andy Whitcroft
> Date: Fri, 13 Sep 2013 17:49:16 +0100
> Subject: [PATCH] scsi: hyper-v storsvc switch up to SPC-3
>
> Suggested-By: James Bottomley
>
> Signed-off-by: Andy Whitcroft
> ---
> drivers/scsi/storvsc_drv.c |8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 9969fa1..3903c8a 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1441,6 +1441,14 @@ static int storvsc_device_configure(struct
> scsi_device *sdevice)
>
> sdevice->no_write_same = 1;
>
> + /*
> + * hyper-v lies about its capabilities indicating it is only SPC-2
> + * compliant, but actually implements the core SPC-3 features.
> + * If we pretend to be SPC-3, we send RC16 which activates trim and
> + * will query the appropriate VPD pages to enable trim.
> + */
> + sdevice->scsi_level = SCSI_SPC_3;
> +
> return 0;
> }
>
> --
> 1.7.9.5
>
> (Downloaded from
> http://kernel.ubuntu.com/git?p=jsalisbury/stable/trusty/ubuntu-
> trusty.git;a=patch;h=ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d
> ).
>
> I think it's unwise to override the scsi_level at this particular point
> because
> you are going to do it for ALL Hyper-V "disks" (perhaps all Hyper-V SCSI
> devices?)...
I agree with you. I was only referring virtual hard disks (VHDs) being
presented to the guest and
not pass-through devices. Furthermore, we would fix up the scsi conformance
level based on the
version of the host we are running on.
Regards,
K. Y
>
> Here's the SCSI inquiry information reported by a USB 2 hard disk being
> passed passed-through by one of my 2012 R2 hosts:
>
> # sg_inq /dev/sdc
> standard INQUIRY:
> PQual=0 Device_type=0 RMB=0 version=0x02 [SCSI-2]
> [AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=1
> SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0]
> EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0
> [RelAdr=0] WBus16=0 Sync=0 Linked=0 [TranDis=0] CmdQue=0
> length=36 (0x24) Peripheral device type: disk
> Vendor identification: MDT MD50
> Product identification: 00AAKS-00TMA0
> Product revision level:
>
> Is it OK to replace a scsi_level of SCSI-2 with SCSI_SPC_3? Additionally is it
> also OK to force SCSI_SPC_3 on Hyper-V 2008?
>
> > > > NryزXvؖ){nlj{zX}zj:v zZzf~zwڢ)jyA
> > > >
> > > > i
> > N?r??yb?X??ǧv?^?){.n?+{zX????ܨ}???Ơz?&j:+v???
> zZ+??+zf???h???~i???z? ?w?&?)ߢf??^jǫy?m??@A?a??
> ?
>
> 0??h???i
>
> ^^^ Where do these characters come from? I've occasionally seen them on
> emails from other Microsoft folks posting to LKML too...
>
> --
> Sitsofe | http://sucs.org/~sits/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] scsi: storvsc: Force SPC-3 for Win8 Hosts or Later
> -Original Message-
> From: Jeff Leung [mailto:jle...@v10networks.ca]
> Sent: Monday, October 20, 2014 10:39 PM
> To: linux-s...@vger.kernel.org; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org
> Cc: KY Srinivasan; james.bottom...@hansenpartnership.com; Jeff Leung
> Subject: [PATCH] scsi: storvsc: Force SPC-3 for Win8 Hosts or Later
>
> This patch forces SPC-3 for Hyper-V disks on the VMBus. As Windows 10's
> virtual SAS bus is SPC-3 compliant and there are no negative side effects on
> forcing SPC-3 compliance for Win8 hosts, this patch enables SPC-3 compliance
> by forcing it on for hosts with versions later than Win8.
>
> Forcing SPC-3 compliance for hosts earlier than Win10 also enables TRIM
> support.
>
> Suggested by: James Bottomley
>
> Signed-off-by: Jeff Leung
> ---
> drivers/scsi/storvsc_drv.c |9 +
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> ed0f899..afcc68e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1449,6 +1449,15 @@ static int storvsc_device_configure(struct
> scsi_device *sdevice)
>
> sdevice->no_write_same = 1;
>
> + /*
> + * If the host is Win2k12 or later, we pretend to be SPC-3 compliant
> + * and send RC16 which activates TRIM. We will only enable this on a
> + * host with levels greater than VERSION_WIN8
> + */
> + if (vmbus_proto_version >= VERSION_WIN8) {
I would want this hack not to be in Windows 10. We can have this hack if the
host is either
Ws2012 or ws2012 r2 (VERSION_WIN8 or VERSION_WIN8_1). Also this hack should
apply only
to VHD's being presented and not pass through devices.
K. Y
> + sdevice->scsi_level = SCSI_SPC_3;
> + }
> +
> return 0;
> }
>
> --
> 1.7.2.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: binder: move to the "real" part of the kernel
On Tue 2014-10-21 16:12:24, Arnd Bergmann wrote: > On Tuesday 21 October 2014 12:36:22 Pavel Machek wrote: > > On Fri 2014-10-17 01:12:21, Greg Kroah-Hartman wrote: > > > On Thu, Oct 16, 2014 at 10:09:04AM -0700, John Stultz wrote: > > > > On Thu, Oct 16, 2014 at 5:47 AM, Greg Kroah-Hartman > > > > wrote: > > > > > From: Greg Kroah-Hartman > > > > > > Are the Android guys comfortable with the ABI stability rules they'll > > > > now face? > > > > > > Just because something is in staging, doesn't mean you don't have to > > > follow the same ABI stability rules as the rest of the kernel. If a > > > change had happened to this code that broke userspace in the past, I > > > would have reverted it. So this should not be anything different from > > > what has been happening inthe past. > > > > Actually, there's big difference. > > > > If Al Viro changes core filesystem in a way that breaks > > staging/binder, binder is broken, and if it can't be fixed... well it > > can't be fixed. > > > > If Al Viro changes core filesystem in a way that breaks > > drivers/binder, Al's change is going to be reverted. > > One might have argued that we'd have to do that already, but the reasons > for doing that with binder in the main kernel are certainly stronger. > > > It is really hard to review without API documentation. Normally, API > > documentation is required for stuff like this. > > > > For example: does it add new files in /proc? > > > > Given that it is stable, can we get rid of binder_debug() and > > especially BINDER_DEBUG_ENTRY stuff? > > Good point. We require documentation for every single sysfs attribute > that gets added to a driver (some escape the review, but that doesn't > change the rule), so we should not make an exception for a new procfs > file here. Actually, it looked like it is debugfs file. Code was messy enough that I was not sure. > > Could binder_transcation() be split to smaller functions according to > > CodingStyle? 17 goto targets at the end of function are not exactly > > easy to read. > > > > ginder_thread_read/write also needs splitting. > > Yes, in principle, but this is still a detail that would mainly serve > to simplify review. The problem is more the lack of review and > documentation of the API. Yes, the problem is that code is impossible to review without API documentation. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send()
On 21 October 2014 18:13, Long Li wrote: > Thanks Sitsofe. This should have been fixed by this patch: > http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=f88e67149f97d73c704d6fe6f492edde97463025 > > Can you give it a try? Ah this one went mainline a few days ago so I've already been trying it. Yes it resolves my problem (that system is no longer oopsing on startup). I'd have added my Tested-by but it's already been committed :-) What helped you narrow it down? -- Sitsofe | http://sucs.org/~sits/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: comedi: addi_apci_035: remove driver
On Tuesday, October 21, 2014 8:14 AM, Ian Abbott wrote: > On 20/10/14 18:37, H Hartley Sweeten wrote: >> According to ADDI-DATA, this board was discontinued last year and they >> feel that no further development is needed for this driver. Remove the >> driver from comedi to help with the addi-data cleanup. >> >> Signed-off-by: H Hartley Sweeten >> Cc: Ian Abbott >> Cc: Greg Kroah-Hartman >> --- >> drivers/staging/comedi/Kconfig | 8 - >> drivers/staging/comedi/drivers/Makefile| 1 - >> .../comedi/drivers/addi-data/hwdrv_apci035.c | 482 >> - >> drivers/staging/comedi/drivers/addi_apci_035.c | 129 -- >> 4 files changed, 620 deletions(-) >> delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c >> delete mode 100644 drivers/staging/comedi/drivers/addi_apci_035.c > > An alternative may be to convert it into a very simple driver by > stripping out all the interrupt, timer and watchdog stuff, and just > leave an AI subdevice just with insn_read support. But even that seems > hopelessly broken in this driver - it doesn't seem to bother selecting > the channel or the range, for example. So I've no qualms about removing > it altogether. I don't think this board really "fits" with comedi. It's not actually a data acquisition board. The 4 watchdog timers on the board are used to trigger the 2 relays to indicate alarm conditions. In addition the 1 digital input can trigger the relays or they can be triggered through software. The 1 analog input is connected to a temperature sensor on the board. Looks like it would fit better in the iio subsystem to me. Regardless, ADDI-DATA has discontinued the board. Since the current state of the driver is unusable without external patches from ADDI-DATA that disable the insn checking I think it's better to just drop this driver. Reviewed-by: Ian Abbott Regards, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send()
> -Original Message- > From: Sitsofe Wheeler [mailto:sits...@gmail.com] > Sent: Tuesday, October 21, 2014 5:16 PM > To: Long Li > Cc: David Miller; o...@aepfle.de; net...@vger.kernel.org; > jasow...@redhat.com; linux-ker...@vger.kernel.org; a...@canonical.com; > de...@linuxdriverproject.org; Haiyang Zhang > Subject: Re: [PATCH net 1/1] hyperv: Fix a bug in netvsc_send() > > On 21 October 2014 18:13, Long Li wrote: > > Thanks Sitsofe. This should have been fixed by this patch: > > http://git.kernel.org/cgit/linux/kernel/git/next/linux- > next.git/commit/?id=f88e67149f97d73c704d6fe6f492edde97463025 > > > > Can you give it a try? > > Ah this one went mainline a few days ago so I've already been trying > it. Yes it resolves my problem (that system is no longer oopsing on > startup). I'd have added my Tested-by but it's already been committed > :-) Thank you for the test! - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: binder: move to the "real" part of the kernel
On the topic of maintainers for binder: both Arve Hjønnevåg (a...@android.com) and Riley Andrews (riandr...@android.com) have volunteered to be co-maintainers with Greg. We would also like to make kernel-t...@android.com the maintainer of the whole android directory. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
