Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 01:30:44PM -0400, Nick Krause wrote:
> On Sun, Aug 3, 2014 at 8:28 AM, Sergei Shtylyov
> > On 03-08-2014 6:56, Nicholas Krause wrote:
> >
> >> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
> >> order
> >> to avoid a bug with xhci dequence/enquence functions.
> >
> >
> >dequeue/enqueue?
> >
> >
> >> Signed-off-by: Nicholas Krause
> >> ---
> >> include/linux/scatterlist.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >
> >> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> >> index adae88f..62de7b3 100644
> >> --- a/include/linux/scatterlist.h
> >> +++ b/include/linux/scatterlist.h
> >> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
> >> *sg)
> >> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
> >> unsigned int buflen)
> >> {
> >> -#ifdef CONFIG_DEBUG_SG
> >> +#ifdef !CONFIG_DEBUG_SG
> >
> >
> >Didn't you mean #ifndef instead? I guess you didn't even try to
> > build-test this.
> >
> >
> >> BUG_ON(!virt_addr_valid(buf));
> >> #endif
> >> sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
> >
> >
> > WBR, Sergei
> >
> I am going to stay around and learn more but am going to check my
> patches better as this is
> my fault.
This is something like the fourth time you've said this, and you
still haven't managed to do it. :(
Compile the code. Every. Single. Time.
Test the code. Every. Single. Time.
Not optional, not negotiable.
Hugo.
> Regards Nick
> --
> 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/
--
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
--- You stay in the theatre because you're afraid of having no ---
money? There's irony...
signature.asc
Description: Digital signature
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 01:18:45AM -0400, Nick Krause wrote:
> On Sun, Aug 3, 2014 at 1:02 AM, Mateusz Guzik wrote:
> > On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
> >> On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik wrote:
> >> > On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
> >> >> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
> >> >> order
> >> >> to avoid a bug with xhci dequence/enquence functions.
> >> >>
> >> >> Signed-off-by: Nicholas Krause
> >> >> ---
> >> >> include/linux/scatterlist.h | 2 +-
> >> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> >> >> index adae88f..62de7b3 100644
> >> >> --- a/include/linux/scatterlist.h
> >> >> +++ b/include/linux/scatterlist.h
> >> >> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct
> >> >> scatterlist *sg)
> >> >> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
> >> >> unsigned int buflen)
> >> >> {
> >> >> -#ifdef CONFIG_DEBUG_SG
> >> >> +#ifdef !CONFIG_DEBUG_SG
> >> >> BUG_ON(!virt_addr_valid(buf));
> >> >> #endif
> >> >
> >> > Have you tried compiling this? IIRC you said you would compile your
> >> > stuff, what hapened to that?
> >> >
> >> > What exactly were you trying to achieve? Did this BUG_ON detect a
> >> > problem on your system and now you are trying to silence it?
> >> >
> >> > The change would be wrong even if it compiled since it would just
> >> > execute the assertion only when debug is disabled.
> >> >
> >> > --
> >> > Mateusz Guzik
> >> This is the mailing theme I am getting this from,[xhci] kernel BUG at
> >> include/linux/scatterlist.h:115.
> >> I hope this answers your question about the BUG_ON and yes I did
> >> compile check it with make
> >> M=include/. I also checked usb and usb net directories too.
> >
> > So how have you verified it tests you change? Why didn't you perform a
> > full build?
> >
> > This is a syntax error, I suggest you read up about C preprocessor.
> >
> > Your change attempts to flip the condition. Now virt_addr_valid(buf) is
> > tested only with debug disabled. When you enable debug it is suddenly
> > not tested - definitely does not make sense.
> >
> > I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
> >
> > If you actually read the thread you will note:
> >> Looks like I either need specify valid addresses to sg_set_buf(), or
> >> just make the unit test depend on !CONFIG_DEBUG_SG.
> >
> > 1. It is acknowleged the problem is in the caller
> > 2. There is a suggestion to ensure that the UNIT TEST is not executed if
> > CONFIG_DEBUG_SG is enabled (this part was shortened to "!CONFIG_DEBUG_SG"
> > but nobody claims you can use this in if/if[n]def statements)
> >
> > UNIT TEST as in the thingy which resulted in passing down a buffer
> > failing on this BUG_ON.
> >
> > There is no suggestion to do anything with sg_set_buf itself.
> >
> > You were advised several times to find a simpler project. Also people
> > noted that a "beginner kernel programmer" actually means "seasoned
> > programmer learning the kernel". It is clear you are not a seasoned
> > programmer, so why do you insist on doing kernel work?
> >
> > I can only recommend you play with userspace programs for now. These are
> > much easier to debug and experiment with, not to mention have a lot lower
> > entry point.
> I am really losing my temper with people , when all you do is tell
> me to work on something else and don't even point me to how to build
> test in the kernel tree. Are you stating that your every fucking
> change I have to build the kernel over again, that is a waste of
> time and you known it.
Yes, that is *exactly* what they are telling you. Every. Single.
Change. You *must* compile your change, *and* test it fully. If the
change touches something to do with hardware, you must have that
hardware to test with. If the change touches a filesystem, the minimum
testing (note: that's *minimum*) you should be doing is running
xfstests. Other subsystems will have their own test suites as well.
This (compile, and test thoroughly) is *not* a waste of time. It
prevents you wasting everyone else's time, and it ensures that at
least you have some assurance that the code you've written works
properly. Anything else is just lazy and sloppy, and (quite rightly),
nobody wants code by lazy, sloppy programmers in the kernel.
> Please stop telling me I can do this due to a few mistakes that you
> and the other developers are fucking over doing.
It's not "a few mistakes". You've made a cock-up in pretty much
every single patch I've seen from you. These are sometimes logical
errors like this one -- a few moments thought should have told you
that the change wasn't actually fixing anything. More often, you're
demonstrating *obviously* that you have absolutely no idea about what
the code you've changed
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 01:18:45AM -0400, Nick Krause wrote:
On Sun, Aug 3, 2014 at 1:02 AM, Mateusz Guzik mgu...@redhat.com wrote:
On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik mgu...@redhat.com wrote:
On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct
scatterlist *sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
Have you tried compiling this? IIRC you said you would compile your
stuff, what hapened to that?
What exactly were you trying to achieve? Did this BUG_ON detect a
problem on your system and now you are trying to silence it?
The change would be wrong even if it compiled since it would just
execute the assertion only when debug is disabled.
--
Mateusz Guzik
This is the mailing theme I am getting this from,[xhci] kernel BUG at
include/linux/scatterlist.h:115.
I hope this answers your question about the BUG_ON and yes I did
compile check it with make
M=include/. I also checked usb and usb net directories too.
So how have you verified it tests you change? Why didn't you perform a
full build?
This is a syntax error, I suggest you read up about C preprocessor.
Your change attempts to flip the condition. Now virt_addr_valid(buf) is
tested only with debug disabled. When you enable debug it is suddenly
not tested - definitely does not make sense.
I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
If you actually read the thread you will note:
Looks like I either need specify valid addresses to sg_set_buf(), or
just make the unit test depend on !CONFIG_DEBUG_SG.
1. It is acknowleged the problem is in the caller
2. There is a suggestion to ensure that the UNIT TEST is not executed if
CONFIG_DEBUG_SG is enabled (this part was shortened to !CONFIG_DEBUG_SG
but nobody claims you can use this in if/if[n]def statements)
UNIT TEST as in the thingy which resulted in passing down a buffer
failing on this BUG_ON.
There is no suggestion to do anything with sg_set_buf itself.
You were advised several times to find a simpler project. Also people
noted that a beginner kernel programmer actually means seasoned
programmer learning the kernel. It is clear you are not a seasoned
programmer, so why do you insist on doing kernel work?
I can only recommend you play with userspace programs for now. These are
much easier to debug and experiment with, not to mention have a lot lower
entry point.
I am really losing my temper with people , when all you do is tell
me to work on something else and don't even point me to how to build
test in the kernel tree. Are you stating that your every fucking
change I have to build the kernel over again, that is a waste of
time and you known it.
Yes, that is *exactly* what they are telling you. Every. Single.
Change. You *must* compile your change, *and* test it fully. If the
change touches something to do with hardware, you must have that
hardware to test with. If the change touches a filesystem, the minimum
testing (note: that's *minimum*) you should be doing is running
xfstests. Other subsystems will have their own test suites as well.
This (compile, and test thoroughly) is *not* a waste of time. It
prevents you wasting everyone else's time, and it ensures that at
least you have some assurance that the code you've written works
properly. Anything else is just lazy and sloppy, and (quite rightly),
nobody wants code by lazy, sloppy programmers in the kernel.
Please stop telling me I can do this due to a few mistakes that you
and the other developers are fucking over doing.
It's not a few mistakes. You've made a cock-up in pretty much
every single patch I've seen from you. These are sometimes logical
errors like this one -- a few moments thought should have told you
that the change wasn't actually fixing anything. More often, you're
demonstrating *obviously* that you have absolutely no idea about what
the code you've changed should be doing, or what the effect of the
change you've made actually is.
Many people have tried to tell you, with varying degrees of
helpfulness, verbosity and rudeness, where you are going wrong, and
what
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 01:30:44PM -0400, Nick Krause wrote:
On Sun, Aug 3, 2014 at 8:28 AM, Sergei Shtylyov
On 03-08-2014 6:56, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
order
to avoid a bug with xhci dequence/enquence functions.
dequeue/enqueue?
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
*sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
Didn't you mean #ifndef instead? I guess you didn't even try to
build-test this.
BUG_ON(!virt_addr_valid(buf));
#endif
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
WBR, Sergei
I am going to stay around and learn more but am going to check my
patches better as this is
my fault.
This is something like the fourth time you've said this, and you
still haven't managed to do it. :(
Compile the code. Every. Single. Time.
Test the code. Every. Single. Time.
Not optional, not negotiable.
Hugo.
Regards Nick
--
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/
--
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
--- You stay in the theatre because you're afraid of having no ---
money? There's irony...
signature.asc
Description: Digital signature
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 3, 2014 at 8:28 AM, Sergei Shtylyov
wrote:
> Hello.
>
>
> On 03-08-2014 6:56, Nicholas Krause wrote:
>
>> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
>> order
>> to avoid a bug with xhci dequence/enquence functions.
>
>
>dequeue/enqueue?
>
>
>> Signed-off-by: Nicholas Krause
>> ---
>> include/linux/scatterlist.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
>> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
>> index adae88f..62de7b3 100644
>> --- a/include/linux/scatterlist.h
>> +++ b/include/linux/scatterlist.h
>> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
>> *sg)
>> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
>> unsigned int buflen)
>> {
>> -#ifdef CONFIG_DEBUG_SG
>> +#ifdef !CONFIG_DEBUG_SG
>
>
>Didn't you mean #ifndef instead? I guess you didn't even try to
> build-test this.
>
>
>> BUG_ON(!virt_addr_valid(buf));
>> #endif
>> sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
>
>
> WBR, Sergei
>
I am going to stay around and learn more but am going to check my
patches better as this is
my fault.
Regards Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
Hello.
On 03-08-2014 6:56, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
dequeue/enqueue?
Signed-off-by: Nicholas Krause
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
Didn't you mean #ifndef instead? I guess you didn't even try to build-test
this.
BUG_ON(!virt_addr_valid(buf));
#endif
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 01:27:26AM -0400, Nick Krause wrote:
> On Sun, Aug 3, 2014 at 1:18 AM, Nick Krause wrote:
> > I am really losing my temper with people , when all you do is tell me to
> > work on
> > something else and don't even point me to how to build test in the kernel
> > tree.
> > Are you stating that your every fucking change I have to build the kernel
> > over
> > again, that is a waste of time and you known it. Please stop telling me I
> > can
> > do this due to a few mistakes that you and the other developers are fucking
> > over doing.
>
> My apologizes, I misread the message boards and this was my fault. Please stop
> telling me to move away it's really get on my nerves.
Let's examine some stuff and try to reach some conclusions.
=== last btrfs patch ===
It is unclear what were you trying to achieve.
a) did you want to return -EOPNOTSUPP if FALLOC_FL_ZERO_RANGE or
FALLOC_FL_COLLAPSE_RANGE is encountered?
b) did you want to add support for these?
Given your commit message in the patch I presume a).
So let's examine the code:
=
/* Make sure we aren't being give some crap mode */
if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
return -EOPNOTSUPP;
if (mode & FALLOC_FL_PUNCH_HOLE)
return btrfs_punch_hole(inode, offset, len);
=
If you know C language you can tell this code will error out if any flag
other than FALLOC_FL_KEEP_SIZE or FALLOC_FL_PUNCH_HOLE is specified the
code will error out. So the flags you were concerned about are handled
and there is nothing to fix in that regard.
But let's say you brainfart and misread the statement. That happens to
everyone.
The next step is to test your theory.
So what you do is you write a test program using fallocate and both good
and bad flags and verify the results. You see it errors out for the
flags you were concerned about and works fine for flags used in the
statement. Then you revisit the code and see your brainfart, case
closed.
So now let's take a look at the patch:
=
- if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
+ if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE|
+ FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE))
return -EOPNOTSUPP;
=
Well, FALLOC_FL_PUNCH_HOLE is definitely a supported flag as indicated by:
=
if (mode & FALLOC_FL_PUNCH_HOLE)
return btrfs_punch_hole(inode, offset, len);
=
But in the patch all flags (including FALLOC_FL_PUNCH_HOLE) are |'ed into
one and then used to test supportability. This alone shows the patch
cannot be correct.
=== sg_set_buf patch ===
Misunderstanding of the original mail and brokeness of the idea of the
patch were already stated, so let's get to testing stage. Again,
brainfarts happen and there is nothing to be concerned about.
So you do:
=
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
=
$ make M=include
And do some for some other arguments as well. No failure is seen, you send
the patch.
Well, as asked previously, what makes you think you actually
compile-tested the patch? You came up with some command variations and
assumed they work as you expect them to without any evidence. This is
very dangerous.
Here is what should have happened instead:
You intentionally break sg_set_buf by putting a syntax error into the
function, for instance:
=
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
+ blah blah this will not compile as it is
#ifdef CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
=
Did it break my build?
=
$ make M=include
Building modules, stage 2.
MODPOST 0 modules
=
Not only this didn't break, this does not appear to compile anything.
Now you may conclude maybe you have to try to compile actual users and
try out M=drivers/usb. If it still does not break, you know you could
not possibly hit any users. Once it breaks, you put your actual patch
and see what happens:
=
include/linux/scatterlist.h: In function ‘sg_set_buf’:
include/linux/scatterlist.h:114:8: error: macro names must be identifiers
#ifdef !CONFIG_DEBUG_SG
=
Clearly, the patch is syntactically incorrect.
==
All these steps are obvious to everyone who did real-world programming,
are easy to do and you definitely could perform them yourself no
problem.
You sent a lot of patches, almost all of them were broken in obvious
ways which would be detected by employing actual testing. Some of them
were harder to test in runtime, but then why were you trying to address
some obscure issue?
Most of your patches also show you didn't
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 01:27:26AM -0400, Nick Krause wrote:
On Sun, Aug 3, 2014 at 1:18 AM, Nick Krause xerofo...@gmail.com wrote:
I am really losing my temper with people , when all you do is tell me to
work on
something else and don't even point me to how to build test in the kernel
tree.
Are you stating that your every fucking change I have to build the kernel
over
again, that is a waste of time and you known it. Please stop telling me I
can
do this due to a few mistakes that you and the other developers are fucking
over doing.
My apologizes, I misread the message boards and this was my fault. Please stop
telling me to move away it's really get on my nerves.
Let's examine some stuff and try to reach some conclusions.
=== last btrfs patch ===
It is unclear what were you trying to achieve.
a) did you want to return -EOPNOTSUPP if FALLOC_FL_ZERO_RANGE or
FALLOC_FL_COLLAPSE_RANGE is encountered?
b) did you want to add support for these?
Given your commit message in the patch I presume a).
So let's examine the code:
=
/* Make sure we aren't being give some crap mode */
if (mode ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
return -EOPNOTSUPP;
if (mode FALLOC_FL_PUNCH_HOLE)
return btrfs_punch_hole(inode, offset, len);
=
If you know C language you can tell this code will error out if any flag
other than FALLOC_FL_KEEP_SIZE or FALLOC_FL_PUNCH_HOLE is specified the
code will error out. So the flags you were concerned about are handled
and there is nothing to fix in that regard.
But let's say you brainfart and misread the statement. That happens to
everyone.
The next step is to test your theory.
So what you do is you write a test program using fallocate and both good
and bad flags and verify the results. You see it errors out for the
flags you were concerned about and works fine for flags used in the
statement. Then you revisit the code and see your brainfart, case
closed.
So now let's take a look at the patch:
=
- if (mode ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
+ if (mode ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE|
+ FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE))
return -EOPNOTSUPP;
=
Well, FALLOC_FL_PUNCH_HOLE is definitely a supported flag as indicated by:
=
if (mode FALLOC_FL_PUNCH_HOLE)
return btrfs_punch_hole(inode, offset, len);
=
But in the patch all flags (including FALLOC_FL_PUNCH_HOLE) are |'ed into
one and then used to test supportability. This alone shows the patch
cannot be correct.
=== sg_set_buf patch ===
Misunderstanding of the original mail and brokeness of the idea of the
patch were already stated, so let's get to testing stage. Again,
brainfarts happen and there is nothing to be concerned about.
So you do:
=
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
=
$ make M=include
And do some for some other arguments as well. No failure is seen, you send
the patch.
Well, as asked previously, what makes you think you actually
compile-tested the patch? You came up with some command variations and
assumed they work as you expect them to without any evidence. This is
very dangerous.
Here is what should have happened instead:
You intentionally break sg_set_buf by putting a syntax error into the
function, for instance:
=
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
+ blah blah this will not compile as it is
#ifdef CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
=
Did it break my build?
=
$ make M=include
Building modules, stage 2.
MODPOST 0 modules
=
Not only this didn't break, this does not appear to compile anything.
Now you may conclude maybe you have to try to compile actual users and
try out M=drivers/usb. If it still does not break, you know you could
not possibly hit any users. Once it breaks, you put your actual patch
and see what happens:
=
include/linux/scatterlist.h: In function ‘sg_set_buf’:
include/linux/scatterlist.h:114:8: error: macro names must be identifiers
#ifdef !CONFIG_DEBUG_SG
=
Clearly, the patch is syntactically incorrect.
==
All these steps are obvious to everyone who did real-world programming,
are easy to do and you definitely could perform them yourself no
problem.
You sent a lot of patches, almost all of them were broken in obvious
ways which would be detected by employing actual testing. Some of them
were harder to test in runtime, but then why were you trying to address
some obscure issue?
Most of your patches also show you didn't perform due
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
Hello.
On 03-08-2014 6:56, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
dequeue/enqueue?
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
Didn't you mean #ifndef instead? I guess you didn't even try to build-test
this.
BUG_ON(!virt_addr_valid(buf));
#endif
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 3, 2014 at 8:28 AM, Sergei Shtylyov
sergei.shtyl...@cogentembedded.com wrote:
Hello.
On 03-08-2014 6:56, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
order
to avoid a bug with xhci dequence/enquence functions.
dequeue/enqueue?
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
*sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
Didn't you mean #ifndef instead? I guess you didn't even try to
build-test this.
BUG_ON(!virt_addr_valid(buf));
#endif
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
WBR, Sergei
I am going to stay around and learn more but am going to check my
patches better as this is
my fault.
Regards Nick
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 3, 2014 at 1:18 AM, Nick Krause wrote:
> On Sun, Aug 3, 2014 at 1:02 AM, Mateusz Guzik wrote:
>> On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
>>> On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik wrote:
>>> > On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
>>> >> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
>>> >> order
>>> >> to avoid a bug with xhci dequence/enquence functions.
>>> >>
>>> >> Signed-off-by: Nicholas Krause
>>> >> ---
>>> >> include/linux/scatterlist.h | 2 +-
>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
>>> >> index adae88f..62de7b3 100644
>>> >> --- a/include/linux/scatterlist.h
>>> >> +++ b/include/linux/scatterlist.h
>>> >> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct
>>> >> scatterlist *sg)
>>> >> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
>>> >> unsigned int buflen)
>>> >> {
>>> >> -#ifdef CONFIG_DEBUG_SG
>>> >> +#ifdef !CONFIG_DEBUG_SG
>>> >> BUG_ON(!virt_addr_valid(buf));
>>> >> #endif
>>> >
>>> > Have you tried compiling this? IIRC you said you would compile your
>>> > stuff, what hapened to that?
>>> >
>>> > What exactly were you trying to achieve? Did this BUG_ON detect a
>>> > problem on your system and now you are trying to silence it?
>>> >
>>> > The change would be wrong even if it compiled since it would just
>>> > execute the assertion only when debug is disabled.
>>> >
>>> > --
>>> > Mateusz Guzik
>>> This is the mailing theme I am getting this from,[xhci] kernel BUG at
>>> include/linux/scatterlist.h:115.
>>> I hope this answers your question about the BUG_ON and yes I did
>>> compile check it with make
>>> M=include/. I also checked usb and usb net directories too.
>>
>> So how have you verified it tests you change? Why didn't you perform a
>> full build?
>>
>> This is a syntax error, I suggest you read up about C preprocessor.
>>
>> Your change attempts to flip the condition. Now virt_addr_valid(buf) is
>> tested only with debug disabled. When you enable debug it is suddenly
>> not tested - definitely does not make sense.
>>
>> I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
>>
>> If you actually read the thread you will note:
>>> Looks like I either need specify valid addresses to sg_set_buf(), or
>>> just make the unit test depend on !CONFIG_DEBUG_SG.
>>
>> 1. It is acknowleged the problem is in the caller
>> 2. There is a suggestion to ensure that the UNIT TEST is not executed if
>> CONFIG_DEBUG_SG is enabled (this part was shortened to "!CONFIG_DEBUG_SG"
>> but nobody claims you can use this in if/if[n]def statements)
>>
>> UNIT TEST as in the thingy which resulted in passing down a buffer
>> failing on this BUG_ON.
>>
>> There is no suggestion to do anything with sg_set_buf itself.
>>
>> You were advised several times to find a simpler project. Also people
>> noted that a "beginner kernel programmer" actually means "seasoned
>> programmer learning the kernel". It is clear you are not a seasoned
>> programmer, so why do you insist on doing kernel work?
>>
>> I can only recommend you play with userspace programs for now. These are
>> much easier to debug and experiment with, not to mention have a lot lower
>> entry point.
>>
>> --
>> Mateusz Guzik
> Mateusz,
> I am really losing my temper with people , when all you do is tell me to work
> on
> something else and don't even point me to how to build test in the kernel
> tree.
> Are you stating that your every fucking change I have to build the kernel over
> again, that is a waste of time and you known it. Please stop telling me I can
> do this due to a few mistakes that you and the other developers are fucking
> over doing.
> Regards Nick
Matuesz,
My apologizes, I misread the message boards and this was my fault. Please stop
telling me to move away it's really get on my nerves.
Regards Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 3, 2014 at 1:02 AM, Mateusz Guzik wrote:
> On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
>> On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik wrote:
>> > On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
>> >> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
>> >> to avoid a bug with xhci dequence/enquence functions.
>> >>
>> >> Signed-off-by: Nicholas Krause
>> >> ---
>> >> include/linux/scatterlist.h | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
>> >> index adae88f..62de7b3 100644
>> >> --- a/include/linux/scatterlist.h
>> >> +++ b/include/linux/scatterlist.h
>> >> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
>> >> *sg)
>> >> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
>> >> unsigned int buflen)
>> >> {
>> >> -#ifdef CONFIG_DEBUG_SG
>> >> +#ifdef !CONFIG_DEBUG_SG
>> >> BUG_ON(!virt_addr_valid(buf));
>> >> #endif
>> >
>> > Have you tried compiling this? IIRC you said you would compile your
>> > stuff, what hapened to that?
>> >
>> > What exactly were you trying to achieve? Did this BUG_ON detect a
>> > problem on your system and now you are trying to silence it?
>> >
>> > The change would be wrong even if it compiled since it would just
>> > execute the assertion only when debug is disabled.
>> >
>> > --
>> > Mateusz Guzik
>> This is the mailing theme I am getting this from,[xhci] kernel BUG at
>> include/linux/scatterlist.h:115.
>> I hope this answers your question about the BUG_ON and yes I did
>> compile check it with make
>> M=include/. I also checked usb and usb net directories too.
>
> So how have you verified it tests you change? Why didn't you perform a
> full build?
>
> This is a syntax error, I suggest you read up about C preprocessor.
>
> Your change attempts to flip the condition. Now virt_addr_valid(buf) is
> tested only with debug disabled. When you enable debug it is suddenly
> not tested - definitely does not make sense.
>
> I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
>
> If you actually read the thread you will note:
>> Looks like I either need specify valid addresses to sg_set_buf(), or
>> just make the unit test depend on !CONFIG_DEBUG_SG.
>
> 1. It is acknowleged the problem is in the caller
> 2. There is a suggestion to ensure that the UNIT TEST is not executed if
> CONFIG_DEBUG_SG is enabled (this part was shortened to "!CONFIG_DEBUG_SG"
> but nobody claims you can use this in if/if[n]def statements)
>
> UNIT TEST as in the thingy which resulted in passing down a buffer
> failing on this BUG_ON.
>
> There is no suggestion to do anything with sg_set_buf itself.
>
> You were advised several times to find a simpler project. Also people
> noted that a "beginner kernel programmer" actually means "seasoned
> programmer learning the kernel". It is clear you are not a seasoned
> programmer, so why do you insist on doing kernel work?
>
> I can only recommend you play with userspace programs for now. These are
> much easier to debug and experiment with, not to mention have a lot lower
> entry point.
>
> --
> Mateusz Guzik
Mateusz,
I am really losing my temper with people , when all you do is tell me to work on
something else and don't even point me to how to build test in the kernel tree.
Are you stating that your every fucking change I have to build the kernel over
again, that is a waste of time and you known it. Please stop telling me I can
do this due to a few mistakes that you and the other developers are fucking
over doing.
Regards Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
> On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik wrote:
> > On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
> >> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
> >> to avoid a bug with xhci dequence/enquence functions.
> >>
> >> Signed-off-by: Nicholas Krause
> >> ---
> >> include/linux/scatterlist.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> >> index adae88f..62de7b3 100644
> >> --- a/include/linux/scatterlist.h
> >> +++ b/include/linux/scatterlist.h
> >> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
> >> *sg)
> >> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
> >> unsigned int buflen)
> >> {
> >> -#ifdef CONFIG_DEBUG_SG
> >> +#ifdef !CONFIG_DEBUG_SG
> >> BUG_ON(!virt_addr_valid(buf));
> >> #endif
> >
> > Have you tried compiling this? IIRC you said you would compile your
> > stuff, what hapened to that?
> >
> > What exactly were you trying to achieve? Did this BUG_ON detect a
> > problem on your system and now you are trying to silence it?
> >
> > The change would be wrong even if it compiled since it would just
> > execute the assertion only when debug is disabled.
> >
> > --
> > Mateusz Guzik
> This is the mailing theme I am getting this from,[xhci] kernel BUG at
> include/linux/scatterlist.h:115.
> I hope this answers your question about the BUG_ON and yes I did
> compile check it with make
> M=include/. I also checked usb and usb net directories too.
So how have you verified it tests you change? Why didn't you perform a
full build?
This is a syntax error, I suggest you read up about C preprocessor.
Your change attempts to flip the condition. Now virt_addr_valid(buf) is
tested only with debug disabled. When you enable debug it is suddenly
not tested - definitely does not make sense.
I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
If you actually read the thread you will note:
> Looks like I either need specify valid addresses to sg_set_buf(), or
> just make the unit test depend on !CONFIG_DEBUG_SG.
1. It is acknowleged the problem is in the caller
2. There is a suggestion to ensure that the UNIT TEST is not executed if
CONFIG_DEBUG_SG is enabled (this part was shortened to "!CONFIG_DEBUG_SG"
but nobody claims you can use this in if/if[n]def statements)
UNIT TEST as in the thingy which resulted in passing down a buffer
failing on this BUG_ON.
There is no suggestion to do anything with sg_set_buf itself.
You were advised several times to find a simpler project. Also people
noted that a "beginner kernel programmer" actually means "seasoned
programmer learning the kernel". It is clear you are not a seasoned
programmer, so why do you insist on doing kernel work?
I can only recommend you play with userspace programs for now. These are
much easier to debug and experiment with, not to mention have a lot lower
entry point.
--
Mateusz Guzik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik wrote:
> On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
>> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
>> to avoid a bug with xhci dequence/enquence functions.
>>
>> Signed-off-by: Nicholas Krause
>> ---
>> include/linux/scatterlist.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
>> index adae88f..62de7b3 100644
>> --- a/include/linux/scatterlist.h
>> +++ b/include/linux/scatterlist.h
>> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
>> *sg)
>> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
>> unsigned int buflen)
>> {
>> -#ifdef CONFIG_DEBUG_SG
>> +#ifdef !CONFIG_DEBUG_SG
>> BUG_ON(!virt_addr_valid(buf));
>> #endif
>
> Have you tried compiling this? IIRC you said you would compile your
> stuff, what hapened to that?
>
> What exactly were you trying to achieve? Did this BUG_ON detect a
> problem on your system and now you are trying to silence it?
>
> The change would be wrong even if it compiled since it would just
> execute the assertion only when debug is disabled.
>
> --
> Mateusz Guzik
This is the mailing theme I am getting this from,[xhci] kernel BUG at
include/linux/scatterlist.h:115.
I hope this answers your question about the BUG_ON and yes I did
compile check it with make
M=include/. I also checked usb and usb net directories too.
Regards,
Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
> This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
> to avoid a bug with xhci dequence/enquence functions.
>
> Signed-off-by: Nicholas Krause
> ---
> include/linux/scatterlist.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index adae88f..62de7b3 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
> static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
> unsigned int buflen)
> {
> -#ifdef CONFIG_DEBUG_SG
> +#ifdef !CONFIG_DEBUG_SG
> BUG_ON(!virt_addr_valid(buf));
> #endif
Have you tried compiling this? IIRC you said you would compile your
stuff, what hapened to that?
What exactly were you trying to achieve? Did this BUG_ON detect a
problem on your system and now you are trying to silence it?
The change would be wrong even if it compiled since it would just
execute the assertion only when debug is disabled.
--
Mateusz Guzik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
Have you tried compiling this? IIRC you said you would compile your
stuff, what hapened to that?
What exactly were you trying to achieve? Did this BUG_ON detect a
problem on your system and now you are trying to silence it?
The change would be wrong even if it compiled since it would just
execute the assertion only when debug is disabled.
--
Mateusz Guzik
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik mgu...@redhat.com wrote:
On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
*sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
Have you tried compiling this? IIRC you said you would compile your
stuff, what hapened to that?
What exactly were you trying to achieve? Did this BUG_ON detect a
problem on your system and now you are trying to silence it?
The change would be wrong even if it compiled since it would just
execute the assertion only when debug is disabled.
--
Mateusz Guzik
This is the mailing theme I am getting this from,[xhci] kernel BUG at
include/linux/scatterlist.h:115.
I hope this answers your question about the BUG_ON and yes I did
compile check it with make
M=include/. I also checked usb and usb net directories too.
Regards,
Nick
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik mgu...@redhat.com wrote:
On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
*sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
Have you tried compiling this? IIRC you said you would compile your
stuff, what hapened to that?
What exactly were you trying to achieve? Did this BUG_ON detect a
problem on your system and now you are trying to silence it?
The change would be wrong even if it compiled since it would just
execute the assertion only when debug is disabled.
--
Mateusz Guzik
This is the mailing theme I am getting this from,[xhci] kernel BUG at
include/linux/scatterlist.h:115.
I hope this answers your question about the BUG_ON and yes I did
compile check it with make
M=include/. I also checked usb and usb net directories too.
So how have you verified it tests you change? Why didn't you perform a
full build?
This is a syntax error, I suggest you read up about C preprocessor.
Your change attempts to flip the condition. Now virt_addr_valid(buf) is
tested only with debug disabled. When you enable debug it is suddenly
not tested - definitely does not make sense.
I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
If you actually read the thread you will note:
Looks like I either need specify valid addresses to sg_set_buf(), or
just make the unit test depend on !CONFIG_DEBUG_SG.
1. It is acknowleged the problem is in the caller
2. There is a suggestion to ensure that the UNIT TEST is not executed if
CONFIG_DEBUG_SG is enabled (this part was shortened to !CONFIG_DEBUG_SG
but nobody claims you can use this in if/if[n]def statements)
UNIT TEST as in the thingy which resulted in passing down a buffer
failing on this BUG_ON.
There is no suggestion to do anything with sg_set_buf itself.
You were advised several times to find a simpler project. Also people
noted that a beginner kernel programmer actually means seasoned
programmer learning the kernel. It is clear you are not a seasoned
programmer, so why do you insist on doing kernel work?
I can only recommend you play with userspace programs for now. These are
much easier to debug and experiment with, not to mention have a lot lower
entry point.
--
Mateusz Guzik
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 3, 2014 at 1:02 AM, Mateusz Guzik mgu...@redhat.com wrote:
On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik mgu...@redhat.com wrote:
On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct scatterlist
*sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
Have you tried compiling this? IIRC you said you would compile your
stuff, what hapened to that?
What exactly were you trying to achieve? Did this BUG_ON detect a
problem on your system and now you are trying to silence it?
The change would be wrong even if it compiled since it would just
execute the assertion only when debug is disabled.
--
Mateusz Guzik
This is the mailing theme I am getting this from,[xhci] kernel BUG at
include/linux/scatterlist.h:115.
I hope this answers your question about the BUG_ON and yes I did
compile check it with make
M=include/. I also checked usb and usb net directories too.
So how have you verified it tests you change? Why didn't you perform a
full build?
This is a syntax error, I suggest you read up about C preprocessor.
Your change attempts to flip the condition. Now virt_addr_valid(buf) is
tested only with debug disabled. When you enable debug it is suddenly
not tested - definitely does not make sense.
I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
If you actually read the thread you will note:
Looks like I either need specify valid addresses to sg_set_buf(), or
just make the unit test depend on !CONFIG_DEBUG_SG.
1. It is acknowleged the problem is in the caller
2. There is a suggestion to ensure that the UNIT TEST is not executed if
CONFIG_DEBUG_SG is enabled (this part was shortened to !CONFIG_DEBUG_SG
but nobody claims you can use this in if/if[n]def statements)
UNIT TEST as in the thingy which resulted in passing down a buffer
failing on this BUG_ON.
There is no suggestion to do anything with sg_set_buf itself.
You were advised several times to find a simpler project. Also people
noted that a beginner kernel programmer actually means seasoned
programmer learning the kernel. It is clear you are not a seasoned
programmer, so why do you insist on doing kernel work?
I can only recommend you play with userspace programs for now. These are
much easier to debug and experiment with, not to mention have a lot lower
entry point.
--
Mateusz Guzik
Mateusz,
I am really losing my temper with people , when all you do is tell me to work on
something else and don't even point me to how to build test in the kernel tree.
Are you stating that your every fucking change I have to build the kernel over
again, that is a waste of time and you known it. Please stop telling me I can
do this due to a few mistakes that you and the other developers are fucking
over doing.
Regards Nick
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] scatterlist.h: Change CONFIG_DEBUG_SG for ifdef statement in sg_set_bf
On Sun, Aug 3, 2014 at 1:18 AM, Nick Krause xerofo...@gmail.com wrote:
On Sun, Aug 3, 2014 at 1:02 AM, Mateusz Guzik mgu...@redhat.com wrote:
On Sun, Aug 03, 2014 at 12:31:30AM -0400, Nick Krause wrote:
On Sat, Aug 2, 2014 at 11:59 PM, Mateusz Guzik mgu...@redhat.com wrote:
On Sat, Aug 02, 2014 at 10:56:13PM -0400, Nicholas Krause wrote:
This changes the ifdef statement in sg_set_bg to !CONFIG_DEBUG_SG in
order
to avoid a bug with xhci dequence/enquence functions.
Signed-off-by: Nicholas Krause xerofo...@gmail.com
---
include/linux/scatterlist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index adae88f..62de7b3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -111,7 +111,7 @@ static inline struct page *sg_page(struct
scatterlist *sg)
static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
unsigned int buflen)
{
-#ifdef CONFIG_DEBUG_SG
+#ifdef !CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
Have you tried compiling this? IIRC you said you would compile your
stuff, what hapened to that?
What exactly were you trying to achieve? Did this BUG_ON detect a
problem on your system and now you are trying to silence it?
The change would be wrong even if it compiled since it would just
execute the assertion only when debug is disabled.
--
Mateusz Guzik
This is the mailing theme I am getting this from,[xhci] kernel BUG at
include/linux/scatterlist.h:115.
I hope this answers your question about the BUG_ON and yes I did
compile check it with make
M=include/. I also checked usb and usb net directories too.
So how have you verified it tests you change? Why didn't you perform a
full build?
This is a syntax error, I suggest you read up about C preprocessor.
Your change attempts to flip the condition. Now virt_addr_valid(buf) is
tested only with debug disabled. When you enable debug it is suddenly
not tested - definitely does not make sense.
I'm assuming you are talking about https://lkml.org/lkml/2014/7/30/810
If you actually read the thread you will note:
Looks like I either need specify valid addresses to sg_set_buf(), or
just make the unit test depend on !CONFIG_DEBUG_SG.
1. It is acknowleged the problem is in the caller
2. There is a suggestion to ensure that the UNIT TEST is not executed if
CONFIG_DEBUG_SG is enabled (this part was shortened to !CONFIG_DEBUG_SG
but nobody claims you can use this in if/if[n]def statements)
UNIT TEST as in the thingy which resulted in passing down a buffer
failing on this BUG_ON.
There is no suggestion to do anything with sg_set_buf itself.
You were advised several times to find a simpler project. Also people
noted that a beginner kernel programmer actually means seasoned
programmer learning the kernel. It is clear you are not a seasoned
programmer, so why do you insist on doing kernel work?
I can only recommend you play with userspace programs for now. These are
much easier to debug and experiment with, not to mention have a lot lower
entry point.
--
Mateusz Guzik
Mateusz,
I am really losing my temper with people , when all you do is tell me to work
on
something else and don't even point me to how to build test in the kernel
tree.
Are you stating that your every fucking change I have to build the kernel over
again, that is a waste of time and you known it. Please stop telling me I can
do this due to a few mistakes that you and the other developers are fucking
over doing.
Regards Nick
Matuesz,
My apologizes, I misread the message boards and this was my fault. Please stop
telling me to move away it's really get on my nerves.
Regards Nick
--
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/
