Re: Possible locking issue in viotape.c
On Sat, 2007-12-08 at 15:19 -0400, Kevin Winchester wrote: > > Yes, I've used quilt for working with mm patches in the past, but I'm > not too familiar with the mail features. For example, how do you get > the recipient list and Signed-off-by in the patch file? Do you just > edit it by hand? Or is there some guide to quilt I should be reading? > I looked at the guide in /usr/share/doc/quilt/quilt.pdf, but it didn't > have anything about email. /usr/share/doc/quilt/README.MAIL has the > details of all of the mail options, but doesn't give a nice example of > what to do with the patch file and how to call 'quilt mail'. The patch description and Signed-off-by, is added with the command "quilt header -e" which opens and editor.. I usually use something like this to mail out patches 'quilt mail --send --to="[EMAIL PROTECTED]" --cc="linux-kernel@vger.kernel.org"' That's how you add the recipients .. Daniel -- 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: Possible locking issue in viotape.c
Daniel Walker wrote: >> Yes, you are right, I hadn't finished looking at all of the up() calls >> when I came to my conclusion. I will try to convert a few that are not >> on your list, but I would like to know how you are generating your >> patches into those files with the diffstat and recipient list. Is that >> a feature of some git command? > > Git may have a similar feature, but I'm using a tool call Quilt. It's > used for managing a list of patches, and it can automatically add a > diffstat to the patch header (part of the quilt refresh command). I also > use it for emailing a list of patches. > Yes, I've used quilt for working with mm patches in the past, but I'm not too familiar with the mail features. For example, how do you get the recipient list and Signed-off-by in the patch file? Do you just edit it by hand? Or is there some guide to quilt I should be reading? I looked at the guide in /usr/share/doc/quilt/quilt.pdf, but it didn't have anything about email. /usr/share/doc/quilt/README.MAIL has the details of all of the mail options, but doesn't give a nice example of what to do with the patch file and how to call 'quilt mail'. -- Kevin Winchester -- 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: Possible locking issue in viotape.c
On Sat, 2007-12-08 at 14:17 -0400, Kevin Winchester wrote: > Daniel Walker wrote: > > On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote: > >> Daniel Walker wrote: > >>> I've posted all the ones I've done so far .. > >>> > >>> ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/ > >>> > >>> Feel free to review or test them.. I've found it pretty easy to simply > >>> grep for certain class of semaphore usage, check if it's conforming to > >>> the mutex requirements, then convert it or not.. Checking them is > >>> getting to be a habit, so I don't think a list would help me.. However, > >>> someone else might be able to use it.. > >>> > >> Thanks, that helps me not duplicate anything. One of the first ones I > >> was looking at (before your post) was viotape.c, which is in your patch > >> set. However, looking at the uses of the semaphore, I see that on line > >> 409-410 the following code: > >> > >> if (noblock) > >> return count; > >> > >> which seems to ignore the fact that the semaphore has been downed (not > >> to mention the dma buffer and op struct allocations. I think it should be: > >> > >>if (noblock) > >>ret = count; > >>goto free_dma; > >> > >> instead. Do you want to make sure I'm right about that and fold it into > >> your patch? Or have you already submitted your patch (or should it be > >> in a separate patch? Alternatively, I can submit the patch if you don't > >> want to bother with it. > > > > viotape was one of the first I started converting, but later I noticed > > the same thing you found above. I have it commented out of my series for > > that reason .. > > > > I think this noblock path is actually doing what the author intended.. > > There are a few stray up() calls related to event handling and ioctls , > > and I think those are used to release the semaphore.. > > > > Daniel > > > > > > Yes, you are right, I hadn't finished looking at all of the up() calls > when I came to my conclusion. I will try to convert a few that are not > on your list, but I would like to know how you are generating your > patches into those files with the diffstat and recipient list. Is that > a feature of some git command? Git may have a similar feature, but I'm using a tool call Quilt. It's used for managing a list of patches, and it can automatically add a diffstat to the patch header (part of the quilt refresh command). I also use it for emailing a list of patches. Daniel -- 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: Possible locking issue in viotape.c
Daniel Walker wrote: > On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote: >> Daniel Walker wrote: >>> I've posted all the ones I've done so far .. >>> >>> ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/ >>> >>> Feel free to review or test them.. I've found it pretty easy to simply >>> grep for certain class of semaphore usage, check if it's conforming to >>> the mutex requirements, then convert it or not.. Checking them is >>> getting to be a habit, so I don't think a list would help me.. However, >>> someone else might be able to use it.. >>> >> Thanks, that helps me not duplicate anything. One of the first ones I >> was looking at (before your post) was viotape.c, which is in your patch >> set. However, looking at the uses of the semaphore, I see that on line >> 409-410 the following code: >> >> if (noblock) >> return count; >> >> which seems to ignore the fact that the semaphore has been downed (not >> to mention the dma buffer and op struct allocations. I think it should be: >> >> if (noblock) >> ret = count; >> goto free_dma; >> >> instead. Do you want to make sure I'm right about that and fold it into >> your patch? Or have you already submitted your patch (or should it be >> in a separate patch? Alternatively, I can submit the patch if you don't >> want to bother with it. > > viotape was one of the first I started converting, but later I noticed > the same thing you found above. I have it commented out of my series for > that reason .. > > I think this noblock path is actually doing what the author intended.. > There are a few stray up() calls related to event handling and ioctls , > and I think those are used to release the semaphore.. > > Daniel > > Yes, you are right, I hadn't finished looking at all of the up() calls when I came to my conclusion. I will try to convert a few that are not on your list, but I would like to know how you are generating your patches into those files with the diffstat and recipient list. Is that a feature of some git command? -- Kevin Winchester -- 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: Possible locking issue in viotape.c
Daniel Walker wrote: On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote: Daniel Walker wrote: I've posted all the ones I've done so far .. ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/ Feel free to review or test them.. I've found it pretty easy to simply grep for certain class of semaphore usage, check if it's conforming to the mutex requirements, then convert it or not.. Checking them is getting to be a habit, so I don't think a list would help me.. However, someone else might be able to use it.. Thanks, that helps me not duplicate anything. One of the first ones I was looking at (before your post) was viotape.c, which is in your patch set. However, looking at the uses of the semaphore, I see that on line 409-410 the following code: if (noblock) return count; which seems to ignore the fact that the semaphore has been downed (not to mention the dma buffer and op struct allocations. I think it should be: if (noblock) ret = count; goto free_dma; instead. Do you want to make sure I'm right about that and fold it into your patch? Or have you already submitted your patch (or should it be in a separate patch? Alternatively, I can submit the patch if you don't want to bother with it. viotape was one of the first I started converting, but later I noticed the same thing you found above. I have it commented out of my series for that reason .. I think this noblock path is actually doing what the author intended.. There are a few stray up() calls related to event handling and ioctls , and I think those are used to release the semaphore.. Daniel Yes, you are right, I hadn't finished looking at all of the up() calls when I came to my conclusion. I will try to convert a few that are not on your list, but I would like to know how you are generating your patches into those files with the diffstat and recipient list. Is that a feature of some git command? -- Kevin Winchester -- 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: Possible locking issue in viotape.c
On Sat, 2007-12-08 at 14:17 -0400, Kevin Winchester wrote: Daniel Walker wrote: On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote: Daniel Walker wrote: I've posted all the ones I've done so far .. ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/ Feel free to review or test them.. I've found it pretty easy to simply grep for certain class of semaphore usage, check if it's conforming to the mutex requirements, then convert it or not.. Checking them is getting to be a habit, so I don't think a list would help me.. However, someone else might be able to use it.. Thanks, that helps me not duplicate anything. One of the first ones I was looking at (before your post) was viotape.c, which is in your patch set. However, looking at the uses of the semaphore, I see that on line 409-410 the following code: if (noblock) return count; which seems to ignore the fact that the semaphore has been downed (not to mention the dma buffer and op struct allocations. I think it should be: if (noblock) ret = count; goto free_dma; instead. Do you want to make sure I'm right about that and fold it into your patch? Or have you already submitted your patch (or should it be in a separate patch? Alternatively, I can submit the patch if you don't want to bother with it. viotape was one of the first I started converting, but later I noticed the same thing you found above. I have it commented out of my series for that reason .. I think this noblock path is actually doing what the author intended.. There are a few stray up() calls related to event handling and ioctls , and I think those are used to release the semaphore.. Daniel Yes, you are right, I hadn't finished looking at all of the up() calls when I came to my conclusion. I will try to convert a few that are not on your list, but I would like to know how you are generating your patches into those files with the diffstat and recipient list. Is that a feature of some git command? Git may have a similar feature, but I'm using a tool call Quilt. It's used for managing a list of patches, and it can automatically add a diffstat to the patch header (part of the quilt refresh command). I also use it for emailing a list of patches. Daniel -- 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: Possible locking issue in viotape.c
Daniel Walker wrote: Yes, you are right, I hadn't finished looking at all of the up() calls when I came to my conclusion. I will try to convert a few that are not on your list, but I would like to know how you are generating your patches into those files with the diffstat and recipient list. Is that a feature of some git command? Git may have a similar feature, but I'm using a tool call Quilt. It's used for managing a list of patches, and it can automatically add a diffstat to the patch header (part of the quilt refresh command). I also use it for emailing a list of patches. Yes, I've used quilt for working with mm patches in the past, but I'm not too familiar with the mail features. For example, how do you get the recipient list and Signed-off-by in the patch file? Do you just edit it by hand? Or is there some guide to quilt I should be reading? I looked at the guide in /usr/share/doc/quilt/quilt.pdf, but it didn't have anything about email. /usr/share/doc/quilt/README.MAIL has the details of all of the mail options, but doesn't give a nice example of what to do with the patch file and how to call 'quilt mail'. -- Kevin Winchester -- 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: Possible locking issue in viotape.c
On Sat, 2007-12-08 at 15:19 -0400, Kevin Winchester wrote: Yes, I've used quilt for working with mm patches in the past, but I'm not too familiar with the mail features. For example, how do you get the recipient list and Signed-off-by in the patch file? Do you just edit it by hand? Or is there some guide to quilt I should be reading? I looked at the guide in /usr/share/doc/quilt/quilt.pdf, but it didn't have anything about email. /usr/share/doc/quilt/README.MAIL has the details of all of the mail options, but doesn't give a nice example of what to do with the patch file and how to call 'quilt mail'. The patch description and Signed-off-by, is added with the command quilt header -e which opens and editor.. I usually use something like this to mail out patches 'quilt mail --send --to=[EMAIL PROTECTED] --cc=linux-kernel@vger.kernel.org' That's how you add the recipients .. Daniel -- 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: Possible locking issue in viotape.c
On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote: > Daniel Walker wrote: > > > > I've posted all the ones I've done so far .. > > > > ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/ > > > > Feel free to review or test them.. I've found it pretty easy to simply > > grep for certain class of semaphore usage, check if it's conforming to > > the mutex requirements, then convert it or not.. Checking them is > > getting to be a habit, so I don't think a list would help me.. However, > > someone else might be able to use it.. > > > > Thanks, that helps me not duplicate anything. One of the first ones I > was looking at (before your post) was viotape.c, which is in your patch > set. However, looking at the uses of the semaphore, I see that on line > 409-410 the following code: > > if (noblock) > return count; > > which seems to ignore the fact that the semaphore has been downed (not > to mention the dma buffer and op struct allocations. I think it should be: > > if (noblock) > ret = count; > goto free_dma; > > instead. Do you want to make sure I'm right about that and fold it into > your patch? Or have you already submitted your patch (or should it be > in a separate patch? Alternatively, I can submit the patch if you don't > want to bother with it. viotape was one of the first I started converting, but later I noticed the same thing you found above. I have it commented out of my series for that reason .. I think this noblock path is actually doing what the author intended.. There are a few stray up() calls related to event handling and ioctls , and I think those are used to release the semaphore.. Daniel -- 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: Possible locking issue in viotape.c
On Thu, 2007-12-06 at 21:29 -0400, Kevin Winchester wrote: Daniel Walker wrote: I've posted all the ones I've done so far .. ftp://source.mvista.com/pub/dwalker/sem2mutex-2.6.24-rc4/ Feel free to review or test them.. I've found it pretty easy to simply grep for certain class of semaphore usage, check if it's conforming to the mutex requirements, then convert it or not.. Checking them is getting to be a habit, so I don't think a list would help me.. However, someone else might be able to use it.. Thanks, that helps me not duplicate anything. One of the first ones I was looking at (before your post) was viotape.c, which is in your patch set. However, looking at the uses of the semaphore, I see that on line 409-410 the following code: if (noblock) return count; which seems to ignore the fact that the semaphore has been downed (not to mention the dma buffer and op struct allocations. I think it should be: if (noblock) ret = count; goto free_dma; instead. Do you want to make sure I'm right about that and fold it into your patch? Or have you already submitted your patch (or should it be in a separate patch? Alternatively, I can submit the patch if you don't want to bother with it. viotape was one of the first I started converting, but later I noticed the same thing you found above. I have it commented out of my series for that reason .. I think this noblock path is actually doing what the author intended.. There are a few stray up() calls related to event handling and ioctls , and I think those are used to release the semaphore.. Daniel -- 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/
