Re: [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion

[Stefano Garzarella]

Hi Randy,

On Thu, Dec 03, 2020 at 09:37:48AM -0800, Randy Dunlap wrote:
> On 12/3/20 9:04 AM, Stefano Garzarella wrote:
> > Some headers are not necessary, so let's remove them to do
> > some cleaning.
> >
> > Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
>
> Hi,
> What makes you say that some of these are unnecessary?
>
> Please use Rule #1 from Documentation/process/submit-checklist.rst:
>
> 1) If you use a facility then #include the file that defines/declares
>   that facility.  Don't depend on other header files pulling in ones
>   that you use.
>
>
> so just because it will compile without these headers being explictly
> #included does not mean that you should remove them.

Thanks for the clarification. I tried to follow this rule already 
pointed out by Jason, but of course I missed the __init and __exit 
macros...

I'll check better for the next version!

> > ---
> > v3:
> > - avoided to remove some headers with structures and functions directly
> >   used (device.h, slab.h, virtio_byteorder.h)[Jason]
> > ---
> >  drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 ----------
> >  1 file changed, 10 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > index 6a90fdb9cbfc..b08f28d20d8d 100644
> > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > @@ -7,20 +7,10 @@
> >   *
> >   */
> >
> > -#include <linux/init.h>
>
> above is used by __init and __exit.
>
> >  #include <linux/module.h>
> >  #include <linux/device.h>
> > -#include <linux/kernel.h>
> > -#include <linux/fs.h>
> > -#include <linux/poll.h>
>
> Looks OK to remove poll.h.
>
> >  #include <linux/slab.h>
> > -#include <linux/sched.h>
>
> Might be OK for sched.h.
>
> > -#include <linux/wait.h>
>
> Might be OK for wait.h.
>
> > -#include <linux/uuid.h>
> > -#include <linux/iommu.h>
> >  #include <linux/dma-map-ops.h>
> > -#include <linux/sysfs.h>
> > -#include <linux/file.h>
> >  #include <linux/etherdevice.h>
> >  #include <linux/vringh.h>
> >  #include <linux/vdpa.h>
>
> I didn't check the others.

Thanks,
Stefano