Subject: [PATCH 0/4] Rearrange functions to remove forward declarations

This patch series rearranges functions such that forward declarations
becomes unnecessary. Remove those forward declaration.

This patch series is boot tested without user space in qemu with
CONFIG_HW_RANDOM=y.

PrasannaKumar Muralidharan (4):
hw_random: core: Remove forward declaration by rearranging code
hw_random: core: Rearranging rng_get_data to remove forward
declaration
hw_random: core: Rearranging start_khwrngd to remove forward
declaration
hw_random: core: Remove forward declaration of hwrng_init

drivers/char/hw_random/core.c | 151 ++++++++++++++++++++----------------------
1 file changed, 72 insertions(+), 79 deletions(-)

--
2.10.0


Subject: [PATCH 2/4] hw_random: core: Rearranging rng_get_data to remove forward declaration

Rearrange rng_get_data such that its forward declaration is not
required.

Signed-off-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/char/hw_random/core.c | 41 +++++++++++++++++++----------------------
1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 8e1f43c..7e2b1a7 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -52,14 +52,30 @@ MODULE_PARM_DESC(default_quality,
static int hwrng_init(struct hwrng *rng);
static void start_khwrngd(void);

-static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
- int wait);
-
static size_t rng_buffer_size(void)
{
return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
}

+static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
+ int wait) {
+ int present;
+
+ BUG_ON(!mutex_is_locked(&reading_mutex));
+ if (rng->read)
+ return rng->read(rng, (void *)buffer, size, wait);
+
+ if (rng->data_present)
+ present = rng->data_present(rng, wait);
+ else
+ present = 1;
+
+ if (present)
+ return rng->data_read(rng, (u32 *)buffer);
+
+ return 0;
+}
+
static void add_early_randomness(struct hwrng *rng)
{
int bytes_read;
@@ -178,25 +194,6 @@ static int rng_dev_open(struct inode *inode, struct file *filp)
return 0;
}

-static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
- int wait) {
- int present;
-
- BUG_ON(!mutex_is_locked(&reading_mutex));
- if (rng->read)
- return rng->read(rng, (void *)buffer, size, wait);
-
- if (rng->data_present)
- present = rng->data_present(rng, wait);
- else
- present = 1;
-
- if (present)
- return rng->data_read(rng, (u32 *)buffer);
-
- return 0;
-}
-
static ssize_t rng_dev_read(struct file *filp, char __user *buf,
size_t size, loff_t *offp)
{
--
2.10.0

Subject: [PATCH 4/4] hw_random: core: Remove forward declaration of hwrng_init

Rearrange set_current_rng such that hwrng_init's forward declaration can
be removed.

Signed-off-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/char/hw_random/core.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 534c2ae..328d065 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -49,8 +49,6 @@ module_param(default_quality, ushort, 0644);
MODULE_PARM_DESC(default_quality,
"default entropy content of hwrng per mill");

-static int hwrng_init(struct hwrng *rng);
-
static size_t rng_buffer_size(void)
{
return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
@@ -108,22 +106,6 @@ static void drop_current_rng(void)
current_rng = NULL;
}

-static int set_current_rng(struct hwrng *rng)
-{
- int err;
-
- BUG_ON(!mutex_is_locked(&rng_mutex));
-
- err = hwrng_init(rng);
- if (err)
- return err;
-
- drop_current_rng();
- current_rng = rng;
-
- return 0;
-}
-
/* Returns ERR_PTR(), NULL or refcounted hwrng */
static struct hwrng *get_current_rng(void)
{
@@ -220,6 +202,22 @@ static int hwrng_init(struct hwrng *rng)
return 0;
}

+static int set_current_rng(struct hwrng *rng)
+{
+ int err;
+
+ BUG_ON(!mutex_is_locked(&rng_mutex));
+
+ err = hwrng_init(rng);
+ if (err)
+ return err;
+
+ drop_current_rng();
+ current_rng = rng;
+
+ return 0;
+}
+
static int rng_dev_open(struct inode *inode, struct file *filp)
{
/* enforce read-only access to this chrdev */
--
2.10.0

Subject: [PATCH 3/4] hw_random: core: Rearranging start_khwrngd to remove forward declaration

Rearrange start_khwrngd such that its forward declaration is not
required.

Signed-off-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/char/hw_random/core.c | 75 +++++++++++++++++++++----------------------
1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 7e2b1a7..534c2ae 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -50,7 +50,6 @@ MODULE_PARM_DESC(default_quality,
"default entropy content of hwrng per mill");

static int hwrng_init(struct hwrng *rng);
-static void start_khwrngd(void);

static size_t rng_buffer_size(void)
{
@@ -153,6 +152,43 @@ static void put_rng(struct hwrng *rng)
mutex_unlock(&rng_mutex);
}

+static int hwrng_fillfn(void *unused)
+{
+ long rc;
+
+ while (!kthread_should_stop()) {
+ struct hwrng *rng;
+
+ rng = get_current_rng();
+ if (IS_ERR(rng) || !rng)
+ break;
+ mutex_lock(&reading_mutex);
+ rc = rng_get_data(rng, rng_fillbuf,
+ rng_buffer_size(), 1);
+ mutex_unlock(&reading_mutex);
+ put_rng(rng);
+ if (rc <= 0) {
+ pr_warn("hwrng: no data available\n");
+ msleep_interruptible(10000);
+ continue;
+ }
+ /* Outside lock, sure, but y'know: randomness. */
+ add_hwgenerator_randomness((void *)rng_fillbuf, rc,
+ rc * current_quality * 8 >> 10);
+ }
+ hwrng_fill = NULL;
+ return 0;
+}
+
+static void start_khwrngd(void)
+{
+ hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng");
+ if (IS_ERR(hwrng_fill)) {
+ pr_err("hwrng_fill thread creation failed");
+ hwrng_fill = NULL;
+ }
+}
+
static int hwrng_init(struct hwrng *rng)
{
if (kref_get_unless_zero(&rng->ref))
@@ -387,43 +423,6 @@ static int __init register_miscdev(void)
return misc_register(&rng_miscdev);
}

-static int hwrng_fillfn(void *unused)
-{
- long rc;
-
- while (!kthread_should_stop()) {
- struct hwrng *rng;
-
- rng = get_current_rng();
- if (IS_ERR(rng) || !rng)
- break;
- mutex_lock(&reading_mutex);
- rc = rng_get_data(rng, rng_fillbuf,
- rng_buffer_size(), 1);
- mutex_unlock(&reading_mutex);
- put_rng(rng);
- if (rc <= 0) {
- pr_warn("hwrng: no data available\n");
- msleep_interruptible(10000);
- continue;
- }
- /* Outside lock, sure, but y'know: randomness. */
- add_hwgenerator_randomness((void *)rng_fillbuf, rc,
- rc * current_quality * 8 >> 10);
- }
- hwrng_fill = NULL;
- return 0;
-}
-
-static void start_khwrngd(void)
-{
- hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng");
- if (IS_ERR(hwrng_fill)) {
- pr_err("hwrng_fill thread creation failed");
- hwrng_fill = NULL;
- }
-}
-
int hwrng_register(struct hwrng *rng)
{
int err = -EINVAL;
--
2.10.0

Subject: [PATCH 1/4] hw_random: core: Remove forward declaration by rearranging code

Rearrange drop_current_rng such that its forward declaration is not
required.

Signed-off-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/char/hw_random/core.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 9701ac7..8e1f43c 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -49,7 +49,6 @@ module_param(default_quality, ushort, 0644);
MODULE_PARM_DESC(default_quality,
"default entropy content of hwrng per mill");

-static void drop_current_rng(void);
static int hwrng_init(struct hwrng *rng);
static void start_khwrngd(void);

@@ -83,6 +82,17 @@ static inline void cleanup_rng(struct kref *kref)
complete(&rng->cleanup_done);
}

+static void drop_current_rng(void)
+{
+ BUG_ON(!mutex_is_locked(&rng_mutex));
+ if (!current_rng)
+ return;
+
+ /* decrease last reference for triggering the cleanup */
+ kref_put(&current_rng->ref, cleanup_rng);
+ current_rng = NULL;
+}
+
static int set_current_rng(struct hwrng *rng)
{
int err;
@@ -99,17 +109,6 @@ static int set_current_rng(struct hwrng *rng)
return 0;
}

-static void drop_current_rng(void)
-{
- BUG_ON(!mutex_is_locked(&rng_mutex));
- if (!current_rng)
- return;
-
- /* decrease last reference for triggering the cleanup */
- kref_put(&current_rng->ref, cleanup_rng);
- current_rng = NULL;
-}
-
/* Returns ERR_PTR(), NULL or refcounted hwrng */
static struct hwrng *get_current_rng(void)
{
--
2.10.0

2017-10-26 16:48:58

by David Daney

[permalink] [raw]
Subject: Re: [PATCH 0/4] Rearrange functions to remove forward declarations

On 10/26/2017 08:34 AM, PrasannaKumar Muralidharan wrote:
> This patch series rearranges functions such that forward declarations
> becomes unnecessary. Remove those forward declaration.

Why?

The code churn and increase in difficulty of attribution are big
drawbacks to this sort of patch set.


>
> This patch series is boot tested without user space in qemu with
> CONFIG_HW_RANDOM=y.
>
> PrasannaKumar Muralidharan (4):
> hw_random: core: Remove forward declaration by rearranging code
> hw_random: core: Rearranging rng_get_data to remove forward
> declaration
> hw_random: core: Rearranging start_khwrngd to remove forward
> declaration
> hw_random: core: Remove forward declaration of hwrng_init
>
> drivers/char/hw_random/core.c | 151 ++++++++++++++++++++----------------------
> 1 file changed, 72 insertions(+), 79 deletions(-)
>

Subject: Re: [PATCH 0/4] Rearrange functions to remove forward declarations

Hi David,

On 26 October 2017 at 22:18, David Daney <[email protected]> wrote:
> On 10/26/2017 08:34 AM, PrasannaKumar Muralidharan wrote:
>>
>> This patch series rearranges functions such that forward declarations
>> becomes unnecessary. Remove those forward declaration.
>
>
> Why?
>
> The code churn and increase in difficulty of attribution are big drawbacks
> to this sort of patch set.

Completely agreed. I did understand there is little value in this
change. Wasn't sure whether such a patch is encouraged so gave a try.
I am fine with not taking the patch.

Regards,
PrasannaKumar