| Message ID | 1481164052-28036-1-git-send-email-mst@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 12/07/16 18:29, Michael S. Tsirkin wrote: > By now, linux is mostly endian-clean. Enabling endian-ness > checks for everyone produces about 200 new sparse warnings for me - > less than 10% over the 2000 sparse warnings already there. > > Not a big deal, OTOH enabling this helps people notice > they are introducing new bugs. > > So let's just drop __CHECK_ENDIAN__. Follow-up patches > can drop distinction between __bitwise and __bitwise__. Hello Michael, This patch makes a whole bunch of ccflags-y += -D__CHECK_ENDIAN__ statements obsolete. Have you considered to remove these statements? Additionally, there are notable exceptions to the rule that most drivers are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it would remain possible to check such drivers with sparse without enabling endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ into e.g. #ifndef __DONT_CHECK_ENDIAN__? Thanks, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 08, 2016 at 05:21:47AM +0000, Bart Van Assche wrote: > On 12/07/16 18:29, Michael S. Tsirkin wrote: > > By now, linux is mostly endian-clean. Enabling endian-ness > > checks for everyone produces about 200 new sparse warnings for me - > > less than 10% over the 2000 sparse warnings already there. > > > > Not a big deal, OTOH enabling this helps people notice > > they are introducing new bugs. > > > > So let's just drop __CHECK_ENDIAN__. Follow-up patches > > can drop distinction between __bitwise and __bitwise__. > > Hello Michael, > > This patch makes a whole bunch of ccflags-y += -D__CHECK_ENDIAN__ > statements obsolete. Have you considered to remove these statements? Absolutely. Just waiting for feedback on the idea. > Additionally, there are notable exceptions to the rule that most drivers > are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it > would remain possible to check such drivers with sparse without enabling > endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ > into e.g. #ifndef __DONT_CHECK_ENDIAN__? > > Thanks, > > Bart. The right thing is probably just to fix these, isn't it? Until then, why not just ignore the warnings?
On 12/07/16 21:54, Michael S. Tsirkin wrote: > On Thu, Dec 08, 2016 at 05:21:47AM +0000, Bart Van Assche wrote: >> Additionally, there are notable exceptions to the rule that most drivers >> are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it >> would remain possible to check such drivers with sparse without enabling >> endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ >> into e.g. #ifndef __DONT_CHECK_ENDIAN__? > > The right thing is probably just to fix these, isn't it? > Until then, why not just ignore the warnings? Neither option is realistic. With endian-checking enabled the qla2xxx driver triggers so many warnings that it becomes a real challenge to filter the non-endian warnings out manually: $ for f in "" CF=-D__CHECK_ENDIAN__; do make M=drivers/scsi/qla2xxx C=2\ $f | &grep -c ': warning:'; done 4 752 If you think it would be easy to fix the endian warnings triggered by the qla2xxx driver, you are welcome to try to fix these. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 08, 2016 at 06:38:11AM +0000, Bart Van Assche wrote: > On 12/07/16 21:54, Michael S. Tsirkin wrote: > > On Thu, Dec 08, 2016 at 05:21:47AM +0000, Bart Van Assche wrote: > >> Additionally, there are notable exceptions to the rule that most drivers > >> are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it > >> would remain possible to check such drivers with sparse without enabling > >> endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ > >> into e.g. #ifndef __DONT_CHECK_ENDIAN__? > > > > The right thing is probably just to fix these, isn't it? > > Until then, why not just ignore the warnings? > > Neither option is realistic. With endian-checking enabled the qla2xxx > driver triggers so many warnings that it becomes a real challenge to > filter the non-endian warnings out manually: > > $ for f in "" CF=-D__CHECK_ENDIAN__; do make M=drivers/scsi/qla2xxx C=2\ > $f | &grep -c ': warning:'; done > 4 > 752 > > If you think it would be easy to fix the endian warnings triggered by > the qla2xxx driver, you are welcome to try to fix these. Please don't let one crappy, obviously broken, driver prevent this good change from happening which will help ensure that the rest of the kernel (i.e. 99% of it) can be easily tested and fixed for endian issues. Sounds like you should just fix the driver up if it has so many problems, and it annoys you so much that you have to filter out some warnings to see the others :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 8 Dec 2016 04:29:39 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > By now, linux is mostly endian-clean. Enabling endian-ness > checks for everyone produces about 200 new sparse warnings for me - > less than 10% over the 2000 sparse warnings already there. Out of curiousity: Where do most of those warnings show up? > > Not a big deal, OTOH enabling this helps people notice > they are introducing new bugs. > > So let's just drop __CHECK_ENDIAN__. Follow-up patches > can drop distinction between __bitwise and __bitwise__. > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Suggested-by: Christoph Hellwig <hch@infradead.org> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > Linus, could you ack this for upstream? If yes I'll > merge through my tree as a replacement for enabling > this just for virtio. > > include/uapi/linux/types.h | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/include/uapi/linux/types.h b/include/uapi/linux/types.h > index acf0979..41e5914 100644 > --- a/include/uapi/linux/types.h > +++ b/include/uapi/linux/types.h > @@ -23,11 +23,7 @@ > #else > #define __bitwise__ > #endif > -#ifdef __CHECK_ENDIAN__ > #define __bitwise __bitwise__ > -#else > -#define __bitwise > -#endif > > typedef __u16 __bitwise __le16; > typedef __u16 __bitwise __be16; FWIW, I like this better than just enabling it for the virtio code. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 08, 2016 at 06:38:11AM +0000, Bart Van Assche wrote: > On 12/07/16 21:54, Michael S. Tsirkin wrote: > > On Thu, Dec 08, 2016 at 05:21:47AM +0000, Bart Van Assche wrote: > >> Additionally, there are notable exceptions to the rule that most drivers > >> are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it > >> would remain possible to check such drivers with sparse without enabling > >> endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ > >> into e.g. #ifndef __DONT_CHECK_ENDIAN__? > > > > The right thing is probably just to fix these, isn't it? > > Until then, why not just ignore the warnings? > > Neither option is realistic. With endian-checking enabled the qla2xxx > driver triggers so many warnings that it becomes a real challenge to > filter the non-endian warnings out manually: > > $ for f in "" CF=-D__CHECK_ENDIAN__; do make M=drivers/scsi/qla2xxx C=2\ > $f | &grep -c ': warning:'; done > 4 > 752 You can always revert this patch in your tree, or whatever. It does not look like this will get fixed otherwise. > If you think it would be easy to fix the endian warnings triggered by > the qla2xxx driver, you are welcome to try to fix these. > > Bart. Yea, this hardware was designed by someone who thought mixing LE and BE all over the place is a good idea. But who said it should be easy? Maybe this change will be enough to motivate the maintainers. Here's a minor buglet for you as a motivator: if (ct_rsp->header.response != cpu_to_be16(CT_ACCEPT_RESPONSE)) { ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x2077, "%s failed rejected request on port_id: %02x%02x%02x Compeltion status 0x%x, response 0x%x\n", routine, vha->d_id.b.domain, vha->d_id.b.area, vha->d_id.b.al_pa, comp_status, ct_rsp->header.response); response is BE and isn't printed correctly. another: eiter->a.max_frame_size = cpu_to_be32(eiter->a.max_frame_size); size += 4 + 4; ql_dbg(ql_dbg_disc, vha, 0x20bc, "Max_Frame_Size = %x.\n", eiter->a.max_frame_size); printed too late, it's be by that time. Here's another suspicious line ctio24->u.status1.flags = (atio->u.isp24.attr << 9) | cpu_to_le16(CTIO7_FLAGS_STATUS_MODE_1 | CTIO7_FLAGS_TERMINATE); shifting attr by 9 bits gives different results on BE and LE, mixing it with le16 looks rather strange. Another: ha->flags.dport_enabled = (mid_init_cb->init_cb.firmware_options_1 & BIT_7) != 0; BIT_7 is native endian, firmware_options_1 is LE I think. Look at qla27xx_find_valid_image as well. if (pri_image_status.signature != QLA27XX_IMG_STATUS_SIGN) qla27xx_image_status seems to be data coming from flash, but is somehow native-endian? Maybe ... lun = a->u.isp24.fcp_cmnd.lun; I think lun here is in hardware format (le?), code treats it as native. Not to speak about interface abuse all over the place. How about this: uint32_t * qla24xx_read_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr, uint32_t dwords) { uint32_t i; struct qla_hw_data *ha = vha->hw; /* Dword reads to flash. */ for (i = 0; i < dwords; i++, faddr++) dwptr[i] = cpu_to_le32(qla24xx_read_flash_dword(ha, flash_data_addr(ha, faddr))); return dwptr; } OK so we convert to LE ... qla24xx_read_flash_data(vha, dcode, faddr, 4); risc_addr = be32_to_cpu(dcode[2]); *srisc_addr = *srisc_addr == 0 ? risc_addr : *srisc_addr; risc_size = be32_to_cpu(dcode[3]); then happily assume it's BE. And again, coming from flash, it's unlikely to actually be in the native endian-ness as callers seem to assume. I'm guessing it's all BE. I poked at it a bit and was able to cut down # of warnings from 1700 to 1400 in an hour. Someone familiar with the code should look at it.
SGkgTWlrZS9CYXJ0LCANCg0KDQoNCg0KDQoNCg0KT24gMTIvOC8xNiwgODoxNyBBTSwgInZpcnR1 YWxpemF0aW9uLWJvdW5jZXNAbGlzdHMubGludXgtZm91bmRhdGlvbi5vcmcgb24gYmVoYWxmIG9m IE1pY2hhZWwgUy4gVHNpcmtpbiIgPHZpcnR1YWxpemF0aW9uLWJvdW5jZXNAbGlzdHMubGludXgt Zm91bmRhdGlvbi5vcmcgb24gYmVoYWxmIG9mIG1zdEByZWRoYXQuY29tPiB3cm90ZToNCg0KPk9u IFRodSwgRGVjIDA4LCAyMDE2IGF0IDA2OjM4OjExQU0gKzAwMDAsIEJhcnQgVmFuIEFzc2NoZSB3 cm90ZToNCj4+IE9uIDEyLzA3LzE2IDIxOjU0LCBNaWNoYWVsIFMuIFRzaXJraW4gd3JvdGU6DQo+ PiA+IE9uIFRodSwgRGVjIDA4LCAyMDE2IGF0IDA1OjIxOjQ3QU0gKzAwMDAsIEJhcnQgVmFuIEFz c2NoZSB3cm90ZToNCj4+ID4+IEFkZGl0aW9uYWxseSwgdGhlcmUgYXJlIG5vdGFibGUgZXhjZXB0 aW9ucyB0byB0aGUgcnVsZSB0aGF0IG1vc3QgZHJpdmVycw0KPj4gPj4gYXJlIGVuZGlhbi1jbGVh biwgZS5nLiBkcml2ZXJzL3Njc2kvcWxhMnh4eC4gSSB3b3VsZCBhcHByZWNpYXRlIGl0IGlmIGl0 DQo+PiA+PiB3b3VsZCByZW1haW4gcG9zc2libGUgdG8gY2hlY2sgc3VjaCBkcml2ZXJzIHdpdGgg c3BhcnNlIHdpdGhvdXQgZW5hYmxpbmcNCj4+ID4+IGVuZGlhbm5lc3MgY2hlY2tzLiBIYXZlIHlv dSBjb25zaWRlcmVkIHRvIGNoYW5nZSAjaWZkZWYgX19DSEVDS19FTkRJQU5fXw0KPj4gPj4gaW50 byBlLmcuICNpZm5kZWYgX19ET05UX0NIRUNLX0VORElBTl9fPw0KPj4gPg0KPj4gPiBUaGUgcmln aHQgdGhpbmcgaXMgcHJvYmFibHkganVzdCB0byBmaXggdGhlc2UsIGlzbid0IGl0Pw0KPj4gPiBV bnRpbCB0aGVuLCB3aHkgbm90IGp1c3QgaWdub3JlIHRoZSB3YXJuaW5ncz8NCj4+IA0KPj4gTmVp dGhlciBvcHRpb24gaXMgcmVhbGlzdGljLiBXaXRoIGVuZGlhbi1jaGVja2luZyBlbmFibGVkIHRo ZSBxbGEyeHh4IA0KPj4gZHJpdmVyIHRyaWdnZXJzIHNvIG1hbnkgd2FybmluZ3MgdGhhdCBpdCBi ZWNvbWVzIGEgcmVhbCBjaGFsbGVuZ2UgdG8gDQo+PiBmaWx0ZXIgdGhlIG5vbi1lbmRpYW4gd2Fy bmluZ3Mgb3V0IG1hbnVhbGx5Og0KPj4gDQo+PiAkIGZvciBmIGluICIiIENGPS1EX19DSEVDS19F TkRJQU5fXzsgZG8gbWFrZSBNPWRyaXZlcnMvc2NzaS9xbGEyeHh4IEM9MlwNCj4+ICAgICAgJGYg fCAmZ3JlcCAtYyAnOiB3YXJuaW5nOic7IGRvbmUNCj4+IDQNCj4+IDc1Mg0KPg0KPllvdSBjYW4g YWx3YXlzIHJldmVydCB0aGlzIHBhdGNoIGluIHlvdXIgdHJlZSwgb3Igd2hhdGV2ZXIuICBJdCBk b2VzIG5vdA0KPmxvb2sgbGlrZSB0aGlzIHdpbGwgZ2V0IGZpeGVkIG90aGVyd2lzZS4NCj4NCj4+ IElmIHlvdSB0aGluayBpdCB3b3VsZCBiZSBlYXN5IHRvIGZpeCB0aGUgZW5kaWFuIHdhcm5pbmdz IHRyaWdnZXJlZCBieSANCj4+IHRoZSBxbGEyeHh4IGRyaXZlciwgeW91IGFyZSB3ZWxjb21lIHRv IHRyeSB0byBmaXggdGhlc2UuDQo+PiANCj4+IEJhcnQuDQo+DQo+WWVhLCB0aGlzIGhhcmR3YXJl IHdhcyBkZXNpZ25lZCBieSBzb21lb25lIHdobyB0aG91Z2h0IG1peGluZw0KPkxFIGFuZCBCRSBh bGwgb3ZlciB0aGUgcGxhY2UgaXMgYSBnb29kIGlkZWEuDQo+QnV0IHdobyBzYWlkIGl0IHNob3Vs ZCBiZSBlYXN5Pw0KPg0KPk1heWJlIHRoaXMgY2hhbmdlIHdpbGwgYmUgZW5vdWdoIHRvIG1vdGl2 YXRlIHRoZSBtYWludGFpbmVycy4NCj4NCj5IZXJlJ3MgYSBtaW5vciBidWdsZXQgZm9yIHlvdSBh cyBhIG1vdGl2YXRvcjoNCj4NCj4gICAgICAgICAgICAgICAgICAgICAgICBpZiAoY3RfcnNwLT5o ZWFkZXIucmVzcG9uc2UgIT0NCj4gICAgICAgICAgICAgICAgICAgICAgICAgICAgY3B1X3RvX2Jl MTYoQ1RfQUNDRVBUX1JFU1BPTlNFKSkgew0KPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgcWxfZGJnKHFsX2RiZ19kaXNjICsgcWxfZGJnX2J1ZmZlciwgdmhhLCAweDIwNzcsDQo+ICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIiVzIGZhaWxlZCByZWplY3RlZCByZXF1 ZXN0IG9uIHBvcnRfaWQ6ICUwMnglMDJ4JTAyeCBDb21wZWx0aW9uIHN0YXR1cyAweCV4LCByZXNw b25zZSAweCV4XG4iLA0KPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHJvdXRp bmUsIHZoYS0+ZF9pZC5iLmRvbWFpbiwNCj4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICB2aGEtPmRfaWQuYi5hcmVhLCB2aGEtPmRfaWQuYi5hbF9wYSwgY29tcF9zdGF0dXMsIGN0 X3JzcC0+aGVhZGVyLnJlc3BvbnNlKTsNCj4NCj4NCj5yZXNwb25zZSBpcyBCRSBhbmQgaXNuJ3Qg cHJpbnRlZCBjb3JyZWN0bHkuDQo+DQo+YW5vdGhlcjoNCj4NCj4gICAgICAgIGVpdGVyLT5hLm1h eF9mcmFtZV9zaXplID0gY3B1X3RvX2JlMzIoZWl0ZXItPmEubWF4X2ZyYW1lX3NpemUpOw0KPiAg ICAgICAgc2l6ZSArPSA0ICsgNDsNCj4NCj4gICAgICAgIHFsX2RiZyhxbF9kYmdfZGlzYywgdmhh LCAweDIwYmMsDQo+ICAgICAgICAgICAgIk1heF9GcmFtZV9TaXplID0gJXguXG4iLCBlaXRlci0+ YS5tYXhfZnJhbWVfc2l6ZSk7DQo+DQo+cHJpbnRlZCB0b28gbGF0ZSwgaXQncyBiZSBieSB0aGF0 IHRpbWUuDQo+DQo+SGVyZSdzIGFub3RoZXIgc3VzcGljaW91cyBsaW5lDQo+DQo+ICAgICAgICBj dGlvMjQtPnUuc3RhdHVzMS5mbGFncyA9IChhdGlvLT51LmlzcDI0LmF0dHIgPDwgOSkgfA0KPiAg ICAgICAgICAgIGNwdV90b19sZTE2KENUSU83X0ZMQUdTX1NUQVRVU19NT0RFXzEgfA0KPiAgICAg ICAgICAgICAgICBDVElPN19GTEFHU19URVJNSU5BVEUpOw0KPg0KPnNoaWZ0aW5nIGF0dHIgYnkg OSBiaXRzIGdpdmVzIGRpZmZlcmVudCByZXN1bHRzIG9uIEJFIGFuZCBMRSwNCj5taXhpbmcgaXQg d2l0aCBsZTE2IGxvb2tzIHJhdGhlciBzdHJhbmdlLg0KPg0KPkFub3RoZXI6DQo+DQo+ICAgICAg ICAgICAgICAgIGhhLT5mbGFncy5kcG9ydF9lbmFibGVkID0NCj4gICAgICAgICAgICAgICAgICAg IChtaWRfaW5pdF9jYi0+aW5pdF9jYi5maXJtd2FyZV9vcHRpb25zXzEgJiBCSVRfNykgIT0gMDsN Cj4NCj5CSVRfNyBpcyBuYXRpdmUgZW5kaWFuLCBmaXJtd2FyZV9vcHRpb25zXzEgaXMgTEUgSSB0 aGluay4NCj4NCj4NCj4NCj5Mb29rIGF0IHFsYTI3eHhfZmluZF92YWxpZF9pbWFnZSBhcyB3ZWxs Lg0KPg0KPiAgICAgICAgaWYgKHByaV9pbWFnZV9zdGF0dXMuc2lnbmF0dXJlICE9IFFMQTI3WFhf SU1HX1NUQVRVU19TSUdOKQ0KPg0KPnFsYTI3eHhfaW1hZ2Vfc3RhdHVzIHNlZW1zIHRvIGJlIGRh dGEgY29taW5nIGZyb20gZmxhc2gsIGJ1dCBpcw0KPnNvbWVob3cgbmF0aXZlLWVuZGlhbj8gTWF5 YmUgLi4uDQo+DQo+DQo+ICAgICAgICBsdW4gPSBhLT51LmlzcDI0LmZjcF9jbW5kLmx1bjsNCj4N Cj5JIHRoaW5rIGx1biBoZXJlIGlzIGluIGhhcmR3YXJlIGZvcm1hdCAobGU/KSwgY29kZSB0cmVh dHMgaXQNCj5hcyBuYXRpdmUuDQo+DQo+DQo+Tm90IHRvIHNwZWFrIGFib3V0IGludGVyZmFjZSBh YnVzZSBhbGwgb3ZlciB0aGUgcGxhY2UuDQo+SG93IGFib3V0IHRoaXM6DQo+DQo+dWludDMyX3Qg Kg0KPnFsYTI0eHhfcmVhZF9mbGFzaF9kYXRhKHNjc2lfcWxhX2hvc3RfdCAqdmhhLCB1aW50MzJf dCAqZHdwdHIsIHVpbnQzMl90DQo+ZmFkZHIsDQo+ICAgIHVpbnQzMl90IGR3b3JkcykgICAgICAg ICAgICAgICAgICAgICANCj57DQo+ICAgICAgICB1aW50MzJfdCBpOyAgICAgICAgICAgICAgICAg ICAgIA0KPiAgICAgICAgc3RydWN0IHFsYV9od19kYXRhICpoYSA9IHZoYS0+aHc7DQo+ICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIA0KPiAgICAgICAgLyogRHdvcmQgcmVh ZHMgdG8gZmxhc2guICovDQo+ICAgICAgICBmb3IgKGkgPSAwOyBpIDwgZHdvcmRzOyBpKyssIGZh ZGRyKyspDQo+ICAgICAgICAgICAgICAgIGR3cHRyW2ldID0gY3B1X3RvX2xlMzIocWxhMjR4eF9y ZWFkX2ZsYXNoX2R3b3JkKGhhLA0KPiAgICAgICAgICAgICAgICAgICAgZmxhc2hfZGF0YV9hZGRy KGhhLCBmYWRkcikpKTsNCj4NCj4gICAgICAgIHJldHVybiBkd3B0cjsgICAgICAgICAgICAgICAg ICAgDQo+fQ0KPg0KPk9LIHNvIHdlIGNvbnZlcnQgdG8gTEUgLi4uDQo+DQo+ICAgICAgICAgICAg ICAgIHFsYTI0eHhfcmVhZF9mbGFzaF9kYXRhKHZoYSwgZGNvZGUsIGZhZGRyLCA0KTsgDQo+ICAg IA0KPiAgICAgICAgICAgICAgICByaXNjX2FkZHIgPSBiZTMyX3RvX2NwdShkY29kZVsyXSk7DQo+ ICAgICAgICAgICAgICAgICpzcmlzY19hZGRyID0gKnNyaXNjX2FkZHIgPT0gMCA/IHJpc2NfYWRk ciA6ICpzcmlzY19hZGRyOw0KPiAgICAgICAgICAgICAgICByaXNjX3NpemUgPSBiZTMyX3RvX2Nw dShkY29kZVszXSk7DQo+DQo+dGhlbiBoYXBwaWx5IGFzc3VtZSBpdCdzIEJFLg0KPg0KPkFuZCBh Z2FpbiwgY29taW5nIGZyb20gZmxhc2gsIGl0J3MgdW5saWtlbHkgdG8gYWN0dWFsbHkgYmUgaW4g dGhlIG5hdGl2ZQ0KPmVuZGlhbi1uZXNzIGFzIGNhbGxlcnMgc2VlbSB0byBhc3N1bWUuIEknbSBn dWVzc2luZyBpdCdzIGFsbCBCRS4NCj4NCj5JIHBva2VkIGF0IGl0IGEgYml0IGFuZCB3YXMgYWJs ZSB0byBjdXQgZG93biAjIG9mIHdhcm5pbmdzDQo+ZnJvbSAxNzAwIHRvIDE0MDAgaW4gYW4gaG91 ci4gU29tZW9uZSBmYW1pbGlhciB3aXRoIHRoZSBjb2RlDQo+c2hvdWxkIGxvb2sgYXQgaXQuDQoN Cldl4oCZbGwgdGFrZSBhIGxvb2sgYW5kIHNlbmQgcGF0Y2hlcyB0byByZXNvbHZlIHRoZXNlIHdh cm5pbmdzLiANCg0KPg0KPi0tIA0KPk1TVA0KPl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fDQo+VmlydHVhbGl6YXRpb24gbWFpbGluZyBsaXN0DQo+VmlydHVh bGl6YXRpb25AbGlzdHMubGludXgtZm91bmRhdGlvbi5vcmcNCj5odHRwczovL2xpc3RzLmxpbnV4 Zm91bmRhdGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby92aXJ0dWFsaXphdGlvbg0K -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/08/16 22:40, Madhani, Himanshu wrote:
> We’ll take a look and send patches to resolve these warnings.
Thanks!
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 09, 2016 at 03:18:02PM +0000, Bart Van Assche wrote: > On 12/08/16 22:40, Madhani, Himanshu wrote: > > We’ll take a look and send patches to resolve these warnings. > > Thanks! > > Bart. > Sounds good. I posted what I have so far so that you can start from that.
diff --git a/include/uapi/linux/types.h b/include/uapi/linux/types.h index acf0979..41e5914 100644 --- a/include/uapi/linux/types.h +++ b/include/uapi/linux/types.h @@ -23,11 +23,7 @@ #else #define __bitwise__ #endif -#ifdef __CHECK_ENDIAN__ #define __bitwise __bitwise__ -#else -#define __bitwise -#endif typedef __u16 __bitwise __le16; typedef __u16 __bitwise __be16;
By now, linux is mostly endian-clean. Enabling endian-ness checks for everyone produces about 200 new sparse warnings for me - less than 10% over the 2000 sparse warnings already there. Not a big deal, OTOH enabling this helps people notice they are introducing new bugs. So let's just drop __CHECK_ENDIAN__. Follow-up patches can drop distinction between __bitwise and __bitwise__. Cc: Linus Torvalds <torvalds@linux-foundation.org> Suggested-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- Linus, could you ack this for upstream? If yes I'll merge through my tree as a replacement for enabling this just for virtio. include/uapi/linux/types.h | 4 ---- 1 file changed, 4 deletions(-)